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 draw_limb function #5414

Merged
merged 7 commits into from Oct 6, 2021
Merged

Add draw_limb function #5414

merged 7 commits into from Oct 6, 2021

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Jun 20, 2021

Fixes #5253, and includes an example to demonstrate the new functionality. Testing is done implicitly by existing figure tests for GenericMap.draw_limb()

@dstansby dstansby added the No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) label Jun 20, 2021
@dstansby dstansby added this to the 3.1 milestone Jun 20, 2021
@dstansby dstansby requested review from a team as code owners June 20, 2021 15:35
@dstansby dstansby marked this pull request as draft June 20, 2021 15:35
@dstansby dstansby added map Affects the map submodule visualization Affects the visualization submodule labels Jun 20, 2021
@dstansby
Copy link
Member Author

This currently depends on #5417

@nabobalis nabobalis removed this from the 3.1 milestone Aug 21, 2021
@dstansby dstansby force-pushed the draw-limb branch 4 times, most recently from 27267fc to 33929c8 Compare September 24, 2021 15:31
@dstansby dstansby marked this pull request as ready for review September 24, 2021 15:51
@dstansby dstansby added the Needs Review Needs reviews before merge label Sep 24, 2021
Copy link
Member

@ayshih ayshih left a comment

Choose a reason for hiding this comment

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

Given that you've changed draw_limb() to return None for the hidden part of the limb when the limb is entirely visible, you really ought to also change it so that it returns None for the visible part of the limb when the limb is entirely invisible (e.g., for an observer that is on the complete opposite side of the Sun of a Helioprojective map).

Also, you forgot to add a feature changelog entry!

changelog/5414.bugfix.rst Outdated Show resolved Hide resolved
docs/whatsnew/3.1.rst Outdated Show resolved Hide resolved
sunpy/visualization/limb.py Outdated Show resolved Hide resolved
sunpy/visualization/limb.py Outdated Show resolved Hide resolved
sunpy/visualization/limb.py Outdated Show resolved Hide resolved
sunpy/visualization/limb.py Outdated Show resolved Hide resolved
Copy link
Member

@ayshih ayshih left a comment

Choose a reason for hiding this comment

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

The merge conflict with the whatnew needs to be fixed, otherwise this looks good to me

examples/plotting/limb_plotting.py Outdated Show resolved Hide resolved
Co-authored-by: Stuart Mumford <stuart@cadair.com>
@nabobalis nabobalis merged commit 5e745e5 into sunpy:main Oct 6, 2021
@dstansby dstansby deleted the draw-limb branch October 6, 2021 21:18
@dstansby dstansby removed the Needs Review Needs reviews before merge label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
map Affects the map submodule No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) visualization Affects the visualization submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to draw limb with just an observer coordinate
4 participants