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

IO documentation #2627

Merged
merged 5 commits into from Jan 27, 2021
Merged

IO documentation #2627

merged 5 commits into from Jan 27, 2021

Conversation

jlaehne
Copy link
Contributor

@jlaehne jlaehne commented Jan 25, 2021

Description of the change

Some corrections to the IO documentation

  • Mentions of PIL are amended by pillow, and hyperspy uses imageio and not directly PIL/pillow
  • Replaced/removed non working links to format definitions (Image, Ripple, MSA)
  • Further minor changes in the documentation.
  • Pass on **kwds in image reader/writer and extend docstring.

Progress of the PR

  • Change implemented (can be split into several points),
  • Add handling of **kwds,
  • Add tests,
  • ready for review.

@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #2627 (7ececdc) into RELEASE_next_patch (b871e18) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                  Coverage Diff                   @@
##           RELEASE_next_patch    #2627      +/-   ##
======================================================
+ Coverage               76.30%   76.33%   +0.02%     
======================================================
  Files                     202      202              
  Lines                   29641    29640       -1     
  Branches                 6473     6469       -4     
======================================================
+ Hits                    22619    22625       +6     
+ Misses                   5238     5237       -1     
+ Partials                 1784     1778       -6     
Impacted Files Coverage Δ
hyperspy/io_plugins/image.py 83.33% <100.00%> (ø)
hyperspy/drawing/_widgets/circle.py 52.89% <0.00%> (-9.92%) ⬇️
hyperspy/drawing/_widgets/label.py 64.91% <0.00%> (ø)
hyperspy/drawing/_widgets/line2d.py 49.59% <0.00%> (ø)
hyperspy/signal.py 75.88% <0.00%> (+0.07%) ⬆️
hyperspy/signal_tools.py 65.66% <0.00%> (+0.10%) ⬆️
hyperspy/drawing/widget.py 69.24% <0.00%> (+0.12%) ⬆️
hyperspy/defaults_parser.py 64.12% <0.00%> (+0.27%) ⬆️
hyperspy/drawing/figure.py 89.33% <0.00%> (+0.29%) ⬆️
hyperspy/drawing/_widgets/rectangles.py 38.20% <0.00%> (+0.37%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b871e18...7ececdc. Read the comment docs.

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.

Looks good, I have one suggestion for improvement.

file_format : str
The fileformat defined by its extension that is any one supported by
imageio (PIL/pillow), for a list see
https://imageio.readthedocs.io/en/stable/formats.html.
Copy link
Member

Choose a reason for hiding this comment

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

While you are at it, is it worth to add passing kwds to imageio.imwrite and update the docstring/user guide accordingly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just stumbled across the broken links, but you are right it totally makes sense. I will pass on **kwds both for the reader and writer.

Any idea, why the file_format parameter is even there if it is not passed on? Maybe some legacy issue? Normally, the format will be determined automatically, but if wanted the format argument can be passed as keyword argument.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, this is legacy and it is fine to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already adapted that in the updated version.

@ericpre ericpre merged commit 92ebfd6 into hyperspy:RELEASE_next_patch Jan 27, 2021
@jlaehne jlaehne deleted the pillow branch January 27, 2021 22:12
@ericpre ericpre added this to the v1.6.2 milestone Feb 2, 2021
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.

None yet

2 participants