Cleaned up Dimension casting and name lookup #2790
Conversation
philippjfr
added
type: bug
philippjfr
force-pushed
the
dimension_name_lookup
branch
from
5c061f3
to
dc053d6
Compare
|
Ready to review/merge. |
|
So this is a refactor as opposed to a feature or bug fix? |
holoviews/core/dimension.py
Outdated
| @@ -41,6 +41,41 @@ def param_aliases(d): | |||
| return d | |||
|
|
|||
|
|
|||
| def as_dimension(dimension): | |||
There was a problem hiding this comment.
Could this not simply be called dimension?
There was a problem hiding this comment.
Nevermind...I see dimension is frequently used as name even though dim is a lot shorter.
There was a problem hiding this comment.
I think I prefer as_dimension or asdimension, akin to numpy asarray, dimension is far too commonly used as a variable name so it would cause issues and be confusing.
There was a problem hiding this comment.
Can't say I prefer it but if you insist I'll do it.
There was a problem hiding this comment.
It is a lot shorter so I do prefer it..unless you have any specific objections to it?
It also fixes bugs when constructing Datasets with tuple dimensions. |
philippjfr
force-pushed
the
dimension_name_lookup
branch
from
1754576
to
4128090
Compare
|
Looks good. I'll merge when the tests are green. |
The code is currently littered with
isinstance(dim, Dimension)checks, which is not optimal because it's duplicative but more importantly because it does not check for the full range of valid dimension specs which include tuples and dictionaries. This PR adds two utilities one to cast Dimension-like objects to Dimensions if it isn't already one and the other that looks up the name on Dimensions or Dimension-like objects. It then uses these utilities throughout the code.