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

Move map factory examples from reference to a how-to guide #6977

Merged
merged 5 commits into from
May 9, 2023

Conversation

wtbarnes
Copy link
Member

Fixes #6905

@wtbarnes
Copy link
Member Author

These doctests have been marked as SKIP for a while but maybe we should unskip them...

@wtbarnes wtbarnes added this to the 5.0.0 milestone Apr 26, 2023
@nabobalis
Copy link
Contributor

These doctests have been marked as SKIP for a while but maybe we should unskip them...

Do you want to do that here or shall we punt it later?

@wtbarnes
Copy link
Member Author

I'll take a swing at it here. I think now that we're pulling apart the docs into different locations, ensuring the docs don't go stale is even more important.

@wtbarnes wtbarnes marked this pull request as ready for review April 27, 2023 16:24
@wtbarnes wtbarnes requested a review from a team as a code owner April 27, 2023 16:24
@dstansby dstansby removed this from the 5.0.0 milestone Apr 28, 2023
@nabobalis nabobalis added Documentation No Backport A PR that isn't to be backported to any release branch. (To be used as a flag to other maintainers) labels Apr 28, 2023
Comment on lines 17 to 25
>>> import sunpy.map
>>> import sunpy.data.sample
>>> my_map = sunpy.map.Map(sunpy.data.sample.AIA_171_IMAGE) # doctest: +REMOTE_DATA
>>> my_map = sunpy.map.Map('file.fits') # doctest: +SKIP
>>> import pathlib
>>> my_map = sunpy.map.Map(pathlib.Path('file.fits')) # doctest: +SKIP
>>> sub_dir = pathlib.Path('local_dir/sub_dir')
>>> mymap = sunpy.map.Map(sub_dir / 'another_file.fits') # doctest: +SKIP
Copy link
Contributor

@nabobalis nabobalis Apr 28, 2023

Choose a reason for hiding this comment

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

Suggested change
>>> import sunpy.map
>>> import sunpy.data.sample
>>> my_map = sunpy.map.Map(sunpy.data.sample.AIA_171_IMAGE) # doctest: +REMOTE_DATA
>>> my_map = sunpy.map.Map('file.fits') # doctest: +SKIP
>>> import pathlib
>>> my_map = sunpy.map.Map(pathlib.Path('file.fits')) # doctest: +SKIP
>>> sub_dir = pathlib.Path('local_dir/sub_dir')
>>> mymap = sunpy.map.Map(sub_dir / 'another_file.fits') # doctest: +SKIP
>>> from pathlib import Path
>>> import sunpy.map
>>> import sunpy.data.sample
>>> my_map = sunpy.map.Map(sunpy.data.sample.AIA_171_IMAGE) # doctest: +REMOTE_DATA
>>> my_map = sunpy.map.Map('file.fits') # doctest: +SKIP
>>> my_map = sunpy.map.Map(pathlib.Path('file.fits')) # doctest: +SKIP
>>> mymap = sunpy.map.Map(Path('local_dir/sub_dir') / 'another_file.fits') # doctest: +SKIP

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not a fan of imposing this within the how-to or in sample code in general.

Copy link
Member

Choose a reason for hiding this comment

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

Imposing what? I think having the different examples collected line after line as in the suggested change makes for easier reading.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree placing the import statements above the code is preferable. I don't like the spacing requirements that we typically impose in the rest of the codebase.

Directory containing FITS files
---------------------------------

If there is more than one FITS file in the directory, this will return a list of Map objects.
Copy link
Contributor

@nabobalis nabobalis Apr 28, 2023

Choose a reason for hiding this comment

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

We could use the sample data for this and some of the other examples below?

It would need to use some pathlib to get it work but could be useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not opposed though I worry about cluttering up the example code too much.

@wtbarnes wtbarnes force-pushed the migrate-map-factory-examples branch from a2d7fc3 to 84bd7e5 Compare May 1, 2023 15:41
@wtbarnes wtbarnes added this to the 5.0.0 milestone May 3, 2023
docs/how_to/create_a_map.rst Show resolved Hide resolved
docs/how_to/create_a_map.rst Outdated Show resolved Hide resolved
docs/how_to/create_a_map.rst Outdated Show resolved Hide resolved
docs/how_to/create_a_map.rst Outdated Show resolved Hide resolved
Comment on lines 17 to 25
>>> import sunpy.map
>>> import sunpy.data.sample
>>> my_map = sunpy.map.Map(sunpy.data.sample.AIA_171_IMAGE) # doctest: +REMOTE_DATA
>>> my_map = sunpy.map.Map('file.fits') # doctest: +SKIP
>>> import pathlib
>>> my_map = sunpy.map.Map(pathlib.Path('file.fits')) # doctest: +SKIP
>>> sub_dir = pathlib.Path('local_dir/sub_dir')
>>> mymap = sunpy.map.Map(sub_dir / 'another_file.fits') # doctest: +SKIP
Copy link
Member

Choose a reason for hiding this comment

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

Imposing what? I think having the different examples collected line after line as in the suggested change makes for easier reading.

docs/how_to/create_a_map.rst Show resolved Hide resolved
docs/how_to/create_a_map.rst Show resolved Hide resolved
docs/how_to/create_a_map.rst Show resolved Hide resolved
docs/how_to/create_a_map.rst Outdated Show resolved Hide resolved
docs/how_to/create_a_map.rst Outdated Show resolved Hide resolved
@nabobalis nabobalis merged commit bfabd4e into sunpy:main May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

Move Map factory examples from API docs to a how-to guide
3 participants