| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
So if this is scalarmappables than that I think should be the title, but I'm a bit confused about what's happening w/ categorical here especially given scatter categorical color is a pretty big use case for extending units to scalarmappable. Not that this PR has to do that, but I'm concerned about this PR preventing it.
Sorry, something went wrong.
| def _check_axis(axis): | ||
| if axis is None: | ||
| raise units.ConversionError( | ||
| "Categorical data does not support unit conversion that is not " | ||
| "attached to an x or y Axis." | ||
| ) |
There was a problem hiding this comment.
huh? Does this mean no categorical color? Technically, categorical should work just fine w/ NoNorm (and a colorbar where the labels are in the middle rather than the edges).
Sorry, something went wrong.
There was a problem hiding this comment.
This means no categorical color in this PR, because I didn't dive into the error it gave me... you raise a good point though that this PR should as a minimum not prevent categorical colorbars in the future. Let me take a look again at some point later this week, and I'll ping you with what I find.
Sorry, something went wrong.
There was a problem hiding this comment.
What's the error? Just curious if it would be the same issue for #27706
Sorry, something went wrong.
There was a problem hiding this comment.
It was
RuntimeError: <matplotlib.category.StrCategoryConverter object at 0x126bcd610> failed when trying to return the default units for this image. This may be because support has not been implemented for `axis=None` in the default_units() method.because StrCategoryConverter.default_units() actually uses the axis argument (unlike all our other conversion interfaces.
I think this is fixable though, not sure why I just put those errors in... Don't have time now, but will chase categorical support next time I come back to this.
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Hmm, wondering if we can make a formatter for this, something gets to: (b/c is also useful for any of the discrete cases)
fig, ax = plt.subplots()
im = ax.imshow(([[0,1,2], [2, 0, 1], [1, 2, 0]]), cmap=mcolors.ListedColormap(["tab:orange", "tab:blue", "tab:green"], 3))
cb = fig.colorbar(im)
cb.set_ticks([1/3, 1, 1+2/3])
cb.set_ticklabels([0,1,2])
Sorry, something went wrong.
There was a problem hiding this comment.
I think to do this as a legend would probably be via helper method that grabs the color->value mapping and generates the handles, something like https://matplotlib.org/devdocs/api/collections_api.html#matplotlib.collections.PathCollection.legend_elements but on ScalarMappable
(Which if you're cool w/ me branching off your PR, I may volunteer to do...)
Sorry, something went wrong.
There was a problem hiding this comment.
Right, this is in a good enough state (== tests passing) now if you want to branch off and play around with the colorbar for categoricals. I'm not intending to change anything there myself in this PR, but would be happy to merge something into this branch/PR if it's not too complicated.
Sorry, something went wrong.
Sorry, something went wrong.
|
The new colormapping pipeline merged (#28658) probably means that it'd be easier to create a new PR than rebase this one, but also should I think make the actual make it support units easier. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR summary
This PR adds machinery for the data array given to a ScalarMappable to have units, and for those units to be used with images and associated colorbars.
Fixes #25062
Fixes #17447
Fixes #19476
Fixes astropy/astropy#11306
Still needs a user facing example adding, but opening to get early feedback and full CI runs.
PR checklist