Handle the empty string as a group name #5131
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbednar
changed the title
jlstevens
reviewed
Nov 15, 2021
holoviews/core/util.py
Outdated
| @@ -346,6 +346,8 @@ def tree_attribute(identifier): | |||
| These custom attributes start with a capitalized character when | |||
| applicable (not applicable to underscore or certain unicode characters) | |||
| """ | |||
| if not identifier: | |||
There was a problem hiding this comment.
This would match anything that is falsey even though it should only ever match the empty string. I would prefer to be explicit, i.e.:
if identifier=='':
There was a problem hiding this comment.
I can update this when I add a unit test or two
There was a problem hiding this comment.
That's fine with me, though the rest of the function will then error if someone were to pass 0 or None; not sure if that's any better behavior, but maybe?
There was a problem hiding this comment.
A 0 or None should be error conditions imho: those shouldn't be passed as identifiers in any situation.
There was a problem hiding this comment.
Right; I just mean that this isn't like a Parameterized object that's actually checking for such things; 0 and None will just error later at some point. Anyway, I'm fine with however you want to handle that!
|
Looks good and after a minor change and a new unit test, all the tests are green. Thanks @jbednar for the PR! Merging |
philippjfr
pushed a commit
that referenced
this pull request
Nov 16, 2021
maximlt
added a commit
that referenced
this pull request
Dec 16, 2021
* Fix Violin matplotlib rendering with non-finite values (#5135) * Handle the empty string as a group name (#5131) * Fix unhandled numpy.round overflow return value in core/util.py:bound_range(...) (#5095) * Core/Util: Fix unhandled np.round overflow return The function numpy.round return for the input np.float64(2.6558061446181644e+295) the output numpy.inf. This should lead to density = full_precision_density. * Core/Util: Supress numpy.round overflow error The function numpy.round produces for the input numpy.float64(2.6558061446181644e+295) an FloatingPointError with the message "overflow encountered in multiply". This error should be suppressed because its an internal computation by holoview. * Use context manager Co-authored-by: [email protected] <[email protected]> Co-authored-by: Philipp Rudiger <[email protected]> * Update Plotting_with_Matplotlib.ipynb (#4983) Ordering of the fig_bounds tuple in the documentation was different from that in e.g. https://github.com/holoviz/holoviews/blob/40977c515dd9837019aaa0e5708773e78809fbe1/holoviews/plotting/mpl/plot.py#L69 * Notimplemented binop (#5073) * layout: Fix __add__ and __radd__ implementation * Add layout.Layoutable as a mirror of overlay.Overlayable * Remove a good deal of duplicated code * Remove broken calls to super().__radd__ where the super class does not implement __radd__ * Return NotImplemented when Layout([x,y]) raises NotImplementedError. This allows correct interoperability with external classes that could themselves define __radd__ as stated by: https://docs.python.org/3/library/constants.html#NotImplemented Fixes #3577 * overlay: deduplicate and fix __mul__ * Return NotImplemented when appropriate * Deduplicate code between 2 non-trivial and almost identical implementations of __mul__ * Fix non-inheritance-friendly type checking with a local import to avoid cyclic dependency Fixes #3577 * Fix matplotlib colorbar labeling for dim expressions (#5137) * Ensure FreehandDraw renders when styles set (#5139) * Fix datetime clipping on RangeXY stream (#5138) * Fix Bars legend error when overlaid with annotation (#5142) * Do not raise deprecated .opts warning for empty groups (#5144) * Fix plotly Bar plots containing nans (#5143) * Fix plotly Bar plots containing nans * Update tests * Preserve cols when overlaying on layout (#5141) * Do not merge partially overlapping Stream callbacks (#5133) * Do not merge partially overlapping Stream callbacks * Add tests * Remove print * Add bounds to the cache_size Parameter (#5105) * Fix broken link in Gridded user guide (#5098) * Pin freetype on Windows due to matplotlib error (#5109) * Fixed typo * import bokeh's version from an internal util module (#5103) * Add current_key property to DynamicMap (#5106) * Validate dimensionality of xarray interface data (#5140) * Support xyzservices.TileProvider as hv.Tiles input (#5062) * implementation * docstring, plotly test * add bokeh tests * CI: Fix before release (#5151) * delay projection comparison to optimize geoviews (#5152) * Test suite maintenance (#5157) Fixed or suppressed warnings issued while running the tests * fix cherry-pick and compatibility with holoviews 1.14 * fix linting * Handle unsigned integer dtype in datashader aggregate operation (#5149) * Fix docs CI build (#5088) * Remove conda-forge from the build channels, post-install awscli and pin jupyter_client * remove pytest usage * add missing comma and fix test * CI: improvements to the docs build (#5161) * Fix for Contours consistent of empty and nonempty paths (#5162) * Switch to the PyData sphinx theme (#5163) * use pydata sphinx theme to build the site * add pooch to the docs dependencies * Add release notes for 1.14.6 and 1.14.7 (#5160) * attempt to fix pip build * micro fixes to the release notes Co-authored-by: James A. Bednar <[email protected]> Co-authored-by: w31t1 <[email protected]> Co-authored-by: [email protected] <[email protected]> Co-authored-by: jenssss <[email protected]> Co-authored-by: Douglas Raillard <[email protected]> Co-authored-by: Maxime Liquet <[email protected]> Co-authored-by: James A. Bednar <[email protected]> Co-authored-by: Martin Fleischmann <[email protected]> Co-authored-by: maximlt <[email protected]> Co-authored-by: Simon Høxbro Hansen <[email protected]>
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5130 by making
hv.utils.capitalizework properly for empty strings and by considering the empty string a tree attribute.