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

Difference image example for the gallery #3627

Merged
merged 12 commits into from
Feb 6, 2020
Merged

Difference image example for the gallery #3627

merged 12 commits into from
Feb 6, 2020

Conversation

jmason86
Copy link
Contributor

@jmason86 jmason86 commented Dec 20, 2019

Description

This is my first gallery example so may have done something wrong, especially with sphinx stuff.

Partially address Issue #3.

@pep8speaks
Copy link

pep8speaks commented Dec 20, 2019

Hello @jmason86! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 65:1: E265 block comment should start with '# '

Comment last updated at 2020-02-05 14:25:43 UTC

@ghost
Copy link

ghost commented Dec 20, 2019

Thanks for the pull request @jmason86! Everything looks great!

examples/map/difference_images.py Outdated Show resolved Hide resolved
examples/map/difference_images.py Outdated Show resolved Hide resolved
examples/map/difference_images.py Outdated Show resolved Hide resolved
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.

Thanks for this @jmason86 it looks great. A few inline comments :)

examples/map/difference_images.py Outdated Show resolved Hide resolved
@ayshih ayshih added Examples Related to the Example Gallery. map Affects the map submodule labels Dec 20, 2019
@jmason86
Copy link
Contributor Author

Note that there are several impediments to doing this with coronagraph data right now due to other issues. Unfortunately, that's the class of data that probably does the most difference imaging.

For example, Issues:

WARNING: SunpyUserWarning: Could not determine coordinate frame from map metadata [sunpy.map.mapbase].

and

ValueError: FITS header values must contain standard printable ASCII characters; 'offset_bias.pro\t1.37, 12/18/14, 631.118V15 18 Jun 1997 MAKE_FITS_HDR2' contains characters not representable in ASCII or non-printable characters.

@nabobalis nabobalis added this to the 1.0.7 milestone Dec 20, 2019
@wtbarnes wtbarnes requested a review from a team December 29, 2019 22:11
examples/map/difference_images.py Outdated Show resolved Hide resolved
examples/map/difference_images.py Show resolved Hide resolved
examples/map/difference_images.py Outdated Show resolved Hide resolved
@nabobalis nabobalis modified the milestones: 1.0.7, 1.0.8 Jan 10, 2020
examples/map/difference_images.py Show resolved Hide resolved
examples/map/difference_images.py Show resolved Hide resolved
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.

Thanks @jmason86 this looks really good. One minor comment 😄

examples/map/difference_images.py Outdated Show resolved Hide resolved
examples/map/difference_images.py Show resolved Hide resolved
examples/map/difference_images.py Show resolved Hide resolved
@nabobalis nabobalis self-requested a review January 29, 2020 17:57
@Cadair
Copy link
Member

Cadair commented Feb 5, 2020

This should probably be squash merged.

@Cadair Cadair requested a review from wtbarnes February 5, 2020 16:02
@wtbarnes wtbarnes merged commit 111369c into sunpy:master Feb 6, 2020
@sunpy-backport
Copy link

sunpy-backport bot commented Feb 6, 2020

The backport to 1.0 failed:

Commits ["bc2205adfda2aaacade15ebac7ac53b594b44b12","11956d656e42184395cc53a7e8d23a8715db8a53","feb8509958e54d2664fdbb81a61b8fba273b72b5","00a20db204b81d21ecc586d99b457d1859049f07","574981075b2f495c2dd77067d1f6a7121a68adcf","440c36983f8ba51cef0c4bc3c032b096722e5e82","d1ca1a3fd4b8e5f10e93e0479300fa57ce5a753c","3d891f8c7ed2ff39ae5272706d97fe851c738a6e","f5820c0dbd49b6e52c96dc069c9aa6b4afc7407c","ded490aed9535239e3dc5b129ba1241ead407e86","f228ad4f2eabf012d8047a581719f31b72da1641","6de893a9ebd8c8e6343e5d8d015b6055bec0b12e"] could not be cherry-picked on top of 1.0

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport 1.0
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick bc2205adfda2aaacade15ebac7ac53b594b44b12 11956d656e42184395cc53a7e8d23a8715db8a53 feb8509958e54d2664fdbb81a61b8fba273b72b5 00a20db204b81d21ecc586d99b457d1859049f07 574981075b2f495c2dd77067d1f6a7121a68adcf 440c36983f8ba51cef0c4bc3c032b096722e5e82 d1ca1a3fd4b8e5f10e93e0479300fa57ce5a753c 3d891f8c7ed2ff39ae5272706d97fe851c738a6e f5820c0dbd49b6e52c96dc069c9aa6b4afc7407c ded490aed9535239e3dc5b129ba1241ead407e86 f228ad4f2eabf012d8047a581719f31b72da1641 6de893a9ebd8c8e6343e5d8d015b6055bec0b12e
# Create a new branch with these backported commits.
git checkout -b backport-3627-to-1.0
# Push it to GitHub.
git push --set-upstream origin backport-3627-to-1.0
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is 1.0 and the compare/head branch is backport-3627-to-1.0.

@sunpy-backport
Copy link

sunpy-backport bot commented Feb 6, 2020

The backport to 1.1 failed:

Commits ["bc2205adfda2aaacade15ebac7ac53b594b44b12","11956d656e42184395cc53a7e8d23a8715db8a53","feb8509958e54d2664fdbb81a61b8fba273b72b5","00a20db204b81d21ecc586d99b457d1859049f07","574981075b2f495c2dd77067d1f6a7121a68adcf","440c36983f8ba51cef0c4bc3c032b096722e5e82","d1ca1a3fd4b8e5f10e93e0479300fa57ce5a753c","3d891f8c7ed2ff39ae5272706d97fe851c738a6e","f5820c0dbd49b6e52c96dc069c9aa6b4afc7407c","ded490aed9535239e3dc5b129ba1241ead407e86","f228ad4f2eabf012d8047a581719f31b72da1641","6de893a9ebd8c8e6343e5d8d015b6055bec0b12e"] could not be cherry-picked on top of 1.1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub.
git fetch
# Create new working tree.
git worktree add .worktrees/backport 1.1
# Navigate to the new directory.
cd .worktrees/backport
# Cherry-pick all the commits of this pull request and resolve the likely conflicts.
git cherry-pick bc2205adfda2aaacade15ebac7ac53b594b44b12 11956d656e42184395cc53a7e8d23a8715db8a53 feb8509958e54d2664fdbb81a61b8fba273b72b5 00a20db204b81d21ecc586d99b457d1859049f07 574981075b2f495c2dd77067d1f6a7121a68adcf 440c36983f8ba51cef0c4bc3c032b096722e5e82 d1ca1a3fd4b8e5f10e93e0479300fa57ce5a753c 3d891f8c7ed2ff39ae5272706d97fe851c738a6e f5820c0dbd49b6e52c96dc069c9aa6b4afc7407c ded490aed9535239e3dc5b129ba1241ead407e86 f228ad4f2eabf012d8047a581719f31b72da1641 6de893a9ebd8c8e6343e5d8d015b6055bec0b12e
# Create a new branch with these backported commits.
git checkout -b backport-3627-to-1.1
# Push it to GitHub.
git push --set-upstream origin backport-3627-to-1.1
# Go back to the original working tree.
cd ../..
# Delete the working tree.
git worktree remove .worktrees/backport

Then, create a pull request where the base branch is 1.1 and the compare/head branch is backport-3627-to-1.1.

@Cadair Cadair added the Still Needs Manual Backport This PR needs manually backporting. label Feb 6, 2020
nabobalis added a commit that referenced this pull request Feb 10, 2020
* Difference image example for the gallery

* Added changelog file

* Updates responding to code review

* Pass keywords to plot() instead of using plot_settings

* Line break to avoid PEP8 >80 character

* Moved sphinx gallery thumbnail command to end to avoid narrative interruption

* Include channel and imager in comment

Co-Authored-By: Will Barnes <[email protected]>

* No need to use plt.get_cmap()

Co-Authored-By: Will Barnes <[email protected]>

* Space between title and byline

Co-Authored-By: Nabil Freij <[email protected]>

* line break for multi-argument function call

Co-Authored-By: Stuart Mumford <[email protected]>

* Update 3627.doc.rst

Co-authored-by: Will Barnes <[email protected]>
Co-authored-by: Nabil Freij <[email protected]>
Co-authored-by: Stuart Mumford <[email protected]>
@nabobalis nabobalis removed the Still Needs Manual Backport This PR needs manually backporting. label Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Examples Related to the Example Gallery. map Affects the map submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants