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

s.save docstring: for write_dataset parameter add zspy #3203

Open
wants to merge 1 commit into
base: RELEASE_next_minor
Choose a base branch
from

Conversation

magnunor
Copy link
Contributor

When working with really large datasets, it is nice to change the metadata without having to rewrite the whole dataset.

Currently, the docstring only mentions .hspy but it seems to work for (at least) the default .zspy (in other words, the folder structure one).

However, I'm not sure if this will work for different zarr readers/writers.

Description of the change

  • Add .zspy to s.save(write_dataset) parameter.

@CSSFrancis

@codecov
Copy link

codecov bot commented Jul 23, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (056670f) 81.22% compared to head (6f12b24) 84.11%.
Report is 666 commits behind head on RELEASE_next_major.

Files Patch % Lines
hyperspy/decorators.py 96.20% 2 Missing and 1 partial ⚠️
hyperspy/drawing/_widgets/horizontal_line.py 66.66% 0 Missing and 1 partial ⚠️
hyperspy/drawing/_widgets/rectangles.py 66.66% 0 Missing and 1 partial ⚠️
hyperspy/drawing/_widgets/vertical_line.py 66.66% 0 Missing and 1 partial ⚠️
hyperspy/drawing/widget.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           RELEASE_next_major    #3203      +/-   ##
======================================================
+ Coverage               81.22%   84.11%   +2.89%     
======================================================
  Files                     173      197      +24     
  Lines                   24164    32718    +8554     
  Branches                 5618     8883    +3265     
======================================================
+ Hits                    19626    27521    +7895     
- Misses                   3247     3615     +368     
- Partials                 1291     1582     +291     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CSSFrancis
Copy link
Member

@magnunor it would be worth adding a test for this just to make sure. I think I've tested it a couple of time with varying results?

@magnunor
Copy link
Contributor Author

Based on rosettaio, it seems like this is already tested for: https://github.com/hyperspy/rosettasciio/blob/main/rsciio/tests/test_hspy.py#L924

I added a sentence about this only working with the default zspy file writer (DirectoryStore).

@ericpre
Copy link
Member

ericpre commented Jul 24, 2023

As part of the split, this docstring needs to be changed, because as it is, it is not maintainable! Either it should refer to the rosettasciio docstring or pull all the arguments from the relevant function in rosettasciio.

@CSSFrancis CSSFrancis deleted the branch hyperspy:RELEASE_next_minor December 22, 2023 14:03
@CSSFrancis CSSFrancis closed this Dec 22, 2023
@ericpre
Copy link
Member

ericpre commented Dec 22, 2023

Re-opening because this has been closed automatically by mistake!

@ericpre ericpre reopened this Dec 22, 2023
@ericpre ericpre changed the base branch from RELEASE_next_major to RELEASE_next_minor December 22, 2023 16:34
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.

None yet

3 participants