Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get units from h5 file first #4

Merged
merged 2 commits into from
Feb 22, 2024
Merged

get units from h5 file first #4

merged 2 commits into from
Feb 22, 2024

Conversation

rettigl
Copy link
Collaborator

@rettigl rettigl commented Feb 22, 2024

First try to get units from h5 file, if that fails use DEFAULT_UNITS

Includes also some code reorganization

closes #3

Copy link
Collaborator

@domna domna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some small comments:

  • I think the error message is already included in the trace when you raise ... from exc. So it might be confusing when it is included in the latest error message
  • I think it's better to use if clauses instead of the try... except blocks as they are typically better to understand when reading the code (but I will refactor the reader a bit anyways when I add igor support and use the multi format reader).

Nothing of this is blocking the merge, so it is approved.

pynxtools_mpes/reader.py Outdated Show resolved Hide resolved
pynxtools_mpes/reader.py Outdated Show resolved Hide resolved
@rettigl rettigl merged commit 50320cf into main Feb 22, 2024
6 checks passed
@rettigl rettigl deleted the read_units_from_h5 branch April 5, 2024 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take units from h5 file when reading
2 participants