-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Implemented redim method on Dimensioned objects #715
Conversation
@@ -210,7 +215,7 @@ def add_dimension(cls, columns, dimension, dim_pos, values, vdim): | |||
@classmethod | |||
def dframe(cls, columns, dimensions): | |||
if dimensions: | |||
return columns.reindex(columns=dimensions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change snuck in and is unrelated, but I've been meaning to commit it for a while anyway.
I've now added unit tests, so the PR is ready for review. |
Looks good to me! |
Ok, it makes sense. It definitely has to be on My only question is why is it called Other than that one comment, I like this PR and am happy to merge. |
I think I agree with you and may even say that the signatures should be the same. Some interfaces like iris also supplying units so those should also be updated. I'll go ahead and make that change. |
Okay, my latest commit has renamed the interface methods from |
703e2a8
to
4595067
Compare
Ah, the push build passes but the pr one didn't? Is it a transient error? |
Looks like it. Restarting. |
So this makes no sense, one of the new unit tests is failing but only the PR build. Both the push build and my local copy work just fine. |
Additionally adds rename method on interfaces
Finally passing, turned out to be my fault. Part of the tests in this PR depended on the other iris PR. |
Ok. Looks good! Merging. |
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. |
This PR adds the
redim
method to easily replace dimensions or certain attributes on dimensions in a nested object (following the proposal in #714). The dimension overrides are supplied as keyword arguments to the method with the keyword matching the existing dimension by name and the value supplying one of three overrides:curve.redim(x='Time')
.curve.redim(x={'range': (0, 10), 'name': 'Time'})
curve.redim(x=hv.Dimension('Time'))
.The objects the redim applies to can be narrowed down with a specification, which is already in use on the
traverse
,map
andselect
method. This allows only applying theredim
to certain objects in three different ways:layout.redim(hv.Curve, x='Time')
layout.redim('Curve.GDP', x='Time')
layout.redim(lambda x: x.vdims[0].name == 'GDP', x='Time')
The implementation exists in two places, on
Dimensioned
and onDataset
. The implementation on Dataset is required because a change to the dimension names has to be reflected in the underlying datasource. Therename
implementation on all the interfaces so far is pretty trivial and I think it's good functionality to have anyway. Overall I'm pretty happy with this and will be very useful to have, as now it'll be possible to quickly rename dimensions and adjust their ranges.I still need to work on the docstrings, docs and add unit tests.