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

Recognise both byte and string object for NXdata tag in NeXus reader #112

Merged
merged 6 commits into from
May 6, 2023

Conversation

ptim0626
Copy link
Contributor

@ptim0626 ptim0626 commented Apr 28, 2023

This PR (originally hyperspy/hyperspy#3137) is a small improvement on the NeXus reader, allowing it to recognise NXdata tag as both byte and string object. This increases flexibility and avoid missing genuine NXdata dataset when its tag is saved as a string object.

Progress of the PR

  • Change implemented (can be split into several points),
  • add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
  • add test,
  • ready for review.

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 🎉

Comparison is base (85918a4) 85.20% compared to head (05f24b0) 85.26%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #112      +/-   ##
==========================================
+ Coverage   85.20%   85.26%   +0.05%     
==========================================
  Files          73       73              
  Lines        8991     9030      +39     
  Branches     2030     2045      +15     
==========================================
+ Hits         7661     7699      +38     
  Misses        871      871              
- Partials      459      460       +1     
Impacted Files Coverage Δ
rsciio/nexus/_api.py 91.63% <100.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@ericpre ericpre left a comment

Choose a reason for hiding this comment

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

Could you please add a test?

rsciio/nexus/_api.py Outdated Show resolved Hide resolved
rsciio/nexus/_api.py Outdated Show resolved Hide resolved
ptim0626 and others added 3 commits May 2, 2023 15:13
@ptim0626
Copy link
Contributor Author

ptim0626 commented May 2, 2023

Could you please add a test?
Yes it is done now.

@ericpre
Copy link
Member

ericpre commented May 6, 2023

pre-commit.ci autofix

@ericpre ericpre added this to the v0.1.0 initial release milestone May 6, 2023
@ericpre ericpre merged commit f2a258d into hyperspy:main May 6, 2023
@ptim0626 ptim0626 deleted the nexus_nxdata_str branch May 30, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants