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

update selection of interpolation methods #80

Merged
merged 1 commit into from
Nov 13, 2021

Conversation

jimustafa
Copy link
Contributor

Attempts to address #28.

  • replace 'none' with blackman

Something else to consider would be making the image data the same as what is used in the current matplotlib gallery example for the interpolation methods. To that effect, I could add a commit to this PR.

@jklymak
Copy link
Member

jklymak commented Oct 30, 2021

I actually am not sure we should advertise these so exhaustively. It takes a lot of space for something that is pretty advanced.

'none' otoh probably should not be dropped?

@jimustafa
Copy link
Contributor Author

It seemed that there was some consensus in #28 that 'none' could be dropped. For clarity, could we list the 18 interpolations that should be demonstrated.

While I agree that a 3x3 grid of 9 common interpolations may be sufficient, that does leave the question of what to put in the resulting open space. Maybe this is best addressed in a separate issue/PR (see #81).

@jklymak
Copy link
Member

jklymak commented Nov 1, 2021

I suspect the right thing to do here is to make sure the pcolormesh objects are rasterized so that the result of "none" does not depend on the pdf viewer. However "none" is very heavily used (for better or worse) so I think it would be odd to not document how it works in the cheat sheet.

@jklymak
Copy link
Member

jklymak commented Nov 9, 2021

@jimustafa are we publishing the artifacts from CI anywhere?

@jimustafa
Copy link
Contributor Author

Yes @jklymak, the cheatsheets and handouts are saved as build artifacts during workflow execution, and published to the gh-pages branch after being successfully built.

- uses: actions/upload-artifact@v2
if: ${{ always() }}
with:
name: build
path: |
cheatsheets.pdf
handout-*.pdf
- name: Publish cheatsheets and handouts
if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/master' }}
uses: peaceiris/actions-gh-pages@v3
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./build/
force_orphan: true

@jklymak
Copy link
Member

jklymak commented Nov 9, 2021

but only if its a merge to master, right?

@jimustafa
Copy link
Contributor Author

While the artifacts are saved for each build, publishing to gh-pages is only on merge to master, yes.

@jklymak
Copy link
Member

jklymak commented Nov 9, 2021

Yeah, but I never know where the artifacts are stored. Probably just me not seeing a button somewhere...

@QuLogic
Copy link
Member

QuLogic commented Nov 9, 2021

They're on the Summary page, e.g., https://github.com/matplotlib/cheatsheets/actions/runs/1433495962

@@ -13,7 +13,7 @@
subplot_kw={'xticks': [], 'yticks': []})
for ax, interp_method in zip(axs.flat, methods):
ax.imshow(Z, interpolation=interp_method, cmap='viridis',
extent=[0,9,0,9])
extent=[0,9,0,9], rasterized=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @jklymak. Is this what you were suggesting?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. Did it work? After suggesting it, I wasn't sure if imshow rasterized, but I'm pretty sure it does.

@jklymak
Copy link
Member

jklymak commented Nov 12, 2021

This looks fine in my viewer. I think rasterizing it is the right way to go to make it clear what the differences are.

interp

OTOH, the reader is definitely going to be confused as to what None/none/nearest all mean in this context. If I were doing this I would have a section:

# Image interpolation (over-sampled)

"nearest"/"none" = default ("antialiased", `None`), "bilinear"; "bicubic"; etc..

and a different section

# Image interpolation (under-sampled)

## `interpolation_stage="data"`:

"antialiased" = default(`None`); "none"; "hanning"; etc (the other low-pass filters don't matter a ton)

## `interpolation_stage="rgba"`:

"antialiased" = default(`None`); "none"; "hanning"; etc (the other low-pass filters don't matter a ton)

the fact of the matter is that for over-sampled images (many more pixels than data) "none" is a perfectly sensible choice. For under-sampled data (fewer pixels than data) "none" is almost always aliased and should be low-passed first.

See https://matplotlib.org/devdocs/gallery/images_contours_and_fields/image_antialiasing.html

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I think this is the right fix for a consistent display.

@timhoffm timhoffm merged commit 67f86da into matplotlib:master Nov 13, 2021
@jimustafa jimustafa deleted the issue-28 branch November 15, 2021 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants