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

Make sure data examples achieve a single task #6384

Merged
merged 3 commits into from Sep 30, 2022
Merged

Make sure data examples achieve a single task #6384

merged 3 commits into from Sep 30, 2022

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Aug 24, 2022 •
edited by wtbarnes

As part of the documentation reorganisation we will have the four types of documentation described at https://diataxis.fr/. One of these is "How-to guides", which are

recipes, directions that guide the reader through the steps to achieve a specific end.

We already have these guides to some extent in our example gallery.

This PR is the first part of work to make sure the files in our example gallery achieve one specific task. I don't think any of the delted content in this PR needs to be moved elsewhere as it's covered by other examples, but in PRs for the other example categories we may want to move some stuff to entries in a Tutorials section.

Fixes #5888

@dstansby dstansby requested a review from a team as a code owner August 24, 2022 14:08
@dstansby dstansby added the Documentation Affects the documentation label Aug 24, 2022
@dstansby dstansby requested a review from a team August 27, 2022 14:16
@dstansby
Copy link
Member Author

Sorry for the reviewer pain, I've updated my commit and force pushed. All your comments have been addressed - might be worth having a particular look at the cutout example, which I've modified quite a bit. Tested and working locally for me.

@nabobalis
Copy link
Contributor

I think a tracking issue for the tutorials folder should be opened once this is merged so we don't forget.

@nabobalis nabobalis 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 Aug 30, 2022
@nabobalis
Copy link
Contributor

Needs a second review but LGTM

@dstansby dstansby mentioned this pull request Aug 30, 2022
Closed
@dstansby dstansby added the Needs Review Needs reviews before merge label Aug 30, 2022
@dstansby dstansby requested a review from a team September 12, 2022 13:00
Cadair
Cadair reviewed Sep 12, 2022
View reviewed changes
examples/acquiring_data/downloading_lascoC2.py
@@ -1,41 +0,0 @@
"""
======================================
Downloading and plotting LASCO C2 data
Copy link
Member

Choose a reason for hiding this comment

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

I get that this isn't doing anything particularly "interesting" but if someone wants to get a C2 image they are going to find this and follow it. What's the harm in keeping it?

Copy link
Member Author

Choose a reason for hiding this comment

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

If someone wants to get an AIA 171 image do we need an example for that? Another one for C3? Another one for solar orbiter EUI FSI 174? For tasks that don't require an instrument specific workflow, to keep the gallery a manageable size we shouldn't make the examples for carrying out those tasks instrument specific.

Cadair
Cadair reviewed Sep 12, 2022
View reviewed changes
Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

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

While I get the desire to be pure to the methodology, I do think we also need to consider the experience of someone scrolling through the gallery, if you make a plot after you search for / download data then you get a nice preview image where someone can say "hey that looks like what I am after" if we always just stop at Fido.fetch then you don't get that.

This is particularly relevant to the JSOC cutout example here.

@dstansby
Copy link
Member Author

if you make a plot after you search for / download data then you get a nice preview image where someone can say "hey that looks like what I am after" if we always just stop at Fido.fetch then you don't get that.

👍 good point - I will go through these and put back in or add new simple plots wherever we download data.

@wtbarnes
Copy link
Member

I would also point out that the section of our dev guide related to writing example gallery entries (https://docs.sunpy.org/en/latest/dev_guide/contents/example_gallery.html) explicitly says we should include an image wherever possible. If we are going to move away from this policy (I don't think we should, it is a "gallery" after all), we need to change this guide as well.

@dstansby
Copy link
Member Author

No, I don't think we should either! This PR now doesn't remove any final images.

wtbarnes
wtbarnes reviewed Sep 29, 2022
View reviewed changes
examples/map_transformations/downloading_hmi.py
Downloading and plotting an HMI magnetogram
===========================================
==============================================
Rotating HMI maps so they're not 'upside-down'
Copy link
Member

Choose a reason for hiding this comment

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

👍

@nabobalis nabobalis removed request for a team September 30, 2022 14:45
@nabobalis nabobalis removed the request for review from Cadair September 30, 2022 14:45
@nabobalis
Copy link
Contributor

nabobalis commented Sep 30, 2022 •
edited

Doc failure on:

/home/runner/work/sunpy/sunpy/docs/guide/data_types/maps.rst:26: WARNING: undefined label: 'sphx_glr_generated_gallery_acquiring_data_2011_06_07_sampledata_overview.py'

@nabobalis nabobalis removed the Needs Review Needs reviews before merge label Sep 30, 2022
@dstansby dstansby requested a review from a team as a code owner September 30, 2022 15:02
@nabobalis nabobalis merged commit 48b44df into sunpy:main Sep 30, 2022
@dstansby dstansby deleted the descope-data-examples branch September 30, 2022 20:03
@Cadair
Copy link
Member

Cadair commented Oct 1, 2022

This should have had two approvals.

Cadair
Cadair requested changes Oct 1, 2022
View reviewed changes
examples/map_transformations/downloading_hmi.py

This example shows how to download a HMI magnetogram data with Fido and make a plot.
This example shows how to rotate a HMI magnetogram, so when you plot it
it appears with solar North puting up, and not upside down!
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it appears with solar North puting up, and not upside down!
it appears with solar North pointing up.

examples/acquiring_data/downloading_cutouts.py
cutouts.plot()

plt.show()
# Next, we will use this map to definte the top right and bottom
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Next, we will use this map to definte the top right and bottom
# Next, we will use the coordinate frame from this map to definte the top right and bottom

examples/acquiring_data/downloading_cutouts.py

plt.show()
# Next, we will use this map to definte the top right and bottom
# left coordinates we want for the submap.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# left coordinates we want for the submap.
# left coordinates we want for the cutout request.

@nabobalis
Copy link
Contributor

This should have had two approvals.

The original version for sure.

@nabobalis nabobalis mentioned this pull request Oct 1, 2022
Merged
@Cadair
Copy link
Member

Cadair commented Oct 3, 2022

The original version of what?

@nabobalis
Copy link
Contributor

The original version of what?

Of this PR

@Cadair
Copy link
Member

Cadair commented Oct 3, 2022

There was another version of this PR? I am very confused.

@nabobalis
Copy link
Contributor

It's ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wtbarnes wtbarnes wtbarnes left review comments

@Cadair Cadair Cadair requested changes

@nabobalis nabobalis nabobalis approved these changes

Assignees
No one assigned
Labels
Documentation Affects the documentation No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) No Changelog Entry Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace the title of the "Downloading and plotting an HMI magnetogram" example with something more clear
4 participants
@dstansby @nabobalis @wtbarnes @Cadair