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

Allow redim.label if no custom label set #1541

Merged
merged 3 commits into from Jun 14, 2017

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Jun 14, 2017

As the title says this allows using redim.relabel to set a custom label as long as the current label is the same as the name, i.e. no custom label has been supplied previously.

@philippjfr philippjfr changed the title from Allow redim.label if not custom label set to Allow redim.label if no custom label set Jun 14, 2017

@@ -158,6 +158,13 @@ def value_format(self, specs=None, **values):
def range(self, specs=None, **values):
return self._redim('range', specs, **values)
def label(self, specs=None, **values):
for k, v in values.items():
dim = self.parent.get_dimension(k)

This comment has been minimized.

@jlstevens

jlstevens Jun 14, 2017

Member

Seems like None is returned if there is no match for dim? In which case a suitable exception/warning should be given if dim==None.

This comment has been minimized.

@philippjfr

philippjfr Jun 14, 2017

Member

redim doesn't validate Dimensions so you can apply it nested datastructures. That said I'd possibly be open to tightening that up when explicit specs are supplied, not in this PR though.

This comment has been minimized.

@jlstevens

jlstevens Jun 14, 2017

Member

Ah right, thanks for the reminder. It is annoying but I can't see a way around that issue right now...

@jlstevens

This comment has been minimized.

Member

jlstevens commented Jun 14, 2017

Looks good! Happy to merge when the tests pass.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Jun 14, 2017

I see a display transient in the 2.7 pr build but otherwise the tests are passing. Merging.

@jlstevens jlstevens merged commit fa5c3ef into master Jun 14, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls First build on master at 78.086%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the redim_label branch Jun 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment