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.

Sign up for GitHub

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

Jump to bottom

Adding ability to pass clip_interval and vmin/vmax to MapSequence.plot() #7253

Merged
merged 5 commits into from Apr 24, 2024
Merged

Adding ability to pass clip_interval and vmin/vmax to MapSequence.plot() #7253

merged 5 commits into from Apr 24, 2024

Conversation

hayesla
Copy link
Member

@hayesla hayesla commented Oct 23, 2023

Addresses #7252

@wtbarnes
Copy link
Member

This is great, though the amount of code we need to repeat both here and in GenericMap.plot is worrying. I don't know of a good way to avoid that though without an overhaul of how we handle visualization.

This should probably get a figure test as well.

@nabobalis nabobalis mentioned this pull request Feb 26, 2024
Open
@nabobalis
Copy link
Contributor

This is great, though the amount of code we need to repeat both here and in GenericMap.plot is worrying. I don't know of a good way to avoid that though without an overhaul of how we handle visualization.

This should probably get a figure test as well.

Have my terrible idea.

nabobalis
nabobalis reviewed Mar 21, 2024
View reviewed changes
sunpy/map/tests/test_mapsequence.py Outdated
Comment on lines 235 to 240
@figure_test
def test_map_sequence_plot_clip_interval(aia171_test_map):
seq = sunpy.map.Map([aia171_test_map, aia171_test_map, aia171_test_map], sequence=True)
animation = seq.plot(cmap='Greys', clip_interval=(1,99.5)*u.percent)
animation._step()
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to check this works and add the hash

@nabobalis nabobalis marked this pull request as ready for review March 21, 2024 17:20
@nabobalis nabobalis requested a review from a team as a code owner March 21, 2024 17:20
@nabobalis nabobalis added 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) Needs Review Needs reviews before merge labels Mar 21, 2024
@nabobalis nabobalis force-pushed the clip_interval_mapsequence branch 2 times, most recently from 8b0e1f0 to 9dd41f6 Compare March 21, 2024 17:41
nabobalis
nabobalis previously approved these changes Mar 21, 2024
View reviewed changes
wtbarnes
wtbarnes approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@wtbarnes wtbarnes left a comment

Choose a reason for hiding this comment

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

We should probably get another review from someone who didn't write most of the code.

@nabobalis nabobalis added Minor Change PR only needs one approval to merge and removed Needs Review Needs reviews before merge labels Apr 24, 2024
@nabobalis nabobalis merged commit bd9f598 into sunpy:main Apr 24, 2024
26 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wtbarnes wtbarnes wtbarnes approved these changes

@nabobalis nabobalis nabobalis left review comments

Assignees
No one assigned
Labels
map Affects the map submodule Minor Change PR only needs one approval to merge No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hayesla @wtbarnes @nabobalis