The Wayback Machine - https://web.archive.org/web/20211017194832/https://github.com/matplotlib/matplotlib/pull/17367
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

BUGFIX: scatter should draw ',' as a single pixel #17367

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@brunobeltran
Copy link
Collaborator

@brunobeltran brunobeltran commented May 8, 2020

PR Summary

Attempt to fix #11460.

Instead of adding a special case to PathCollection, which (IMO, and I am happy to be told I'm wrong about this) has no business knowing that the path it's drawing came from a MarkerStyle at all, let alone that it came from the weird, special-case marker ',', I added a kwarg to _CollectionWithSizes that allows one to specify that the self-same sizes should actually just be ignored. With this in place, Axes.scatter (which does know that we are using MarkerStyle) can pass the appropriate Path to PathCollection (of width one) and ask the PathCollection to ignore the scaling information, which is what that particular marker is supposed to do, I think.

WIP because I can't seem to figure out one last little issue, where what appear to be identical inputs are being sent to Agg's draw_marker and producing two different results (off-by-one pixel).

Test Cases

import matplotlib.pyplot as plt
fig, ax = plt.subplots()
ax.plot(0, 0, marker=',')
plt.savefig('plot_single_pixel.png')
fig, ax = plt.subplots()
ax.scatter(0, 0, marker=',', edgecolors='none')
plt.savefig('scatter_single_pixel.png')

Output

(Screenshots from GIMP to make it more clear exactly what the error is)

plot case (correctly one pixel):
pixel-zoom

scatter case currently in master (many pixels...):
unfix-scatter-zoom

scatter case in current PR (almost correct):
fix-scatter-zoom

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

1 participant