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 contour map method #3909

Merged
merged 3 commits into from
Jul 17, 2020
Merged

Add contour map method #3909

merged 3 commits into from
Jul 17, 2020

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Mar 20, 2020

I'm doing some contouring at the moment, and thought a helper function might be helpful. My current use case is contouring AIA 193 maps to isolate coronal holes, and then plotting those contours on other maps.

Essentially this just uses skimage.measure.contours, but returns the result as SkyCoord objects. Here is what the example I have added to the gallery looks like:

sphx_glr_map_contouring_001

Needs

  • A documentation example
  • What's new
  • Tests

Test ideas:
Sample maps with:

  • No contours
  • One contour

@dstansby dstansby added the map Affects the map submodule label Mar 20, 2020
@dstansby dstansby requested a review from a team as a code owner March 20, 2020 17:27
@dstansby dstansby added this to the 2.0 milestone Mar 25, 2020
@dpshelio
Copy link
Member

I think it's very useful!! Whether in the future we want these contours as RoIs and be able to extract properties based on them on other datasets is something we could discuss beyond this PR. For now, having this available simple as it's now I think is good. 👍 from me!

@dstansby dstansby changed the title Add contour maputil function Add contour map method Apr 1, 2020
@dstansby dstansby requested a review from a team as a code owner April 1, 2020 09:30
@Cadair Cadair modified the milestones: 2.0, 2.1 Apr 15, 2020
@hayesla
Copy link
Member

hayesla commented May 15, 2020

I think just a small test is now needed

@ehsteve
Copy link
Member

ehsteve commented May 15, 2020

I feel like this provides 90% of the functionality of compositemap. I am not complaining! Just another reason to potentially remove compositemap. Ping @Cadair.

Copy link
Member

@mbobra mbobra left a comment

Choose a reason for hiding this comment

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

This is awesome!!!

@dstansby
Copy link
Member Author

Thanks! Since this won't make it in to 2.0 anyway, I'm waiting until I have a bit more time to add tests.

Copy link
Contributor

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I thought we (cadair) were against adding more to the map API?

@dstansby dstansby requested a review from Cadair July 16, 2020 16:48
@dstansby
Copy link
Member Author

I think methods are more pythonic than functions, but happy to go either way if someone wants to make a decision.

@nabobalis nabobalis merged commit 745f4ea into sunpy:master Jul 17, 2020
@dstansby dstansby deleted the contour branch July 17, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
map Affects the map submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants