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

Add missing docstring for to_image #703

Merged
merged 1 commit into from Apr 9, 2019

Conversation

gerritholl
Copy link
Collaborator

Add missing docstring for the to_image function in the satpy.writers.__init__ module.

  • Closes #xxxx
  • Tests added
  • Tests passed
  • Passes git diff origin/master -- "*py" | flake8 --diff
  • Fully documented

Add missing docstring for the ``to_image`` function in the
``satpy.writers.__init__`` module.
@codecov
Copy link

codecov bot commented Apr 8, 2019

Codecov Report

Merging #703 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #703   +/-   ##
=======================================
  Coverage   79.68%   79.68%           
=======================================
  Files         145      145           
  Lines       21191    21191           
=======================================
  Hits        16886    16886           
  Misses       4305     4305
Impacted Files Coverage Δ
satpy/writers/__init__.py 85.82% <ø> (ø) ⬆️

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 f0d1136...7a243a6. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 79.685% when pulling 7a243a6 on gerritholl:add-missing-docsting-to-image into f0d1136 on pytroll:master.

Copy link
Member

@djhoese djhoese left a comment

Choose a reason for hiding this comment

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

@mraspaud isn't this method now obsolete since I think the XRImage class itself does all of this? Or does it not squeeze? Is the squeeze needed?

@gerritholl
Copy link
Collaborator Author

I don't know if it's obsolete. I got here by going through #513 where this function is recommended. If it's obsolete then instead of this PR it should be changed in #513 (and maybe a DeprecrationWarning issued here?).

@mraspaud
Copy link
Member

mraspaud commented Apr 8, 2019

Squeeze might be needed to remove some unneeded, but tbh I don't remember

@djhoese
Copy link
Member

djhoese commented Apr 8, 2019

This is the current stuff that XRImage does to "correct" for various dimension schemes: https://github.com/pytroll/trollimage/blob/master/trollimage/xrimage.py#L189-L220

It doesn't do any squeezing, but I'm wondering if it could be smarter. This may not be the time since this could get complicated (if you have a 5 dimensional DataArray should you be able to make an image out of that?).

@djhoese
Copy link
Member

djhoese commented Apr 9, 2019

Regardless of whether or not this function is needed, the docstring is useful so I'll merge it. We can figure out how images are created at another time.

@djhoese djhoese merged commit 6e76f37 into pytroll:master Apr 9, 2019
@gerritholl gerritholl deleted the add-missing-docsting-to-image branch April 9, 2019 08:35
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

4 participants