Skip to content
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

Add Dimension.label replacing global alias state #1083

Merged
merged 18 commits into from Jan 30, 2017

Conversation

Projects
None yet
3 participants
@philippjfr
Copy link
Contributor

philippjfr commented Jan 26, 2017

Addresses #1001 and supersedes #1075.

@philippjfr philippjfr force-pushed the dimension_aliases_switched branch from 2770e78 to b759400 Jan 26, 2017

@philippjfr philippjfr referenced this pull request Jan 27, 2017

Closed

Dataset aliases #1075

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Jan 27, 2017

This is now passing without any major issues. This needs a lot more unit tests still and @jlstevens promised to write a short tutorial but we're both very happy with how this will work out.

@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Jan 27, 2017

Looks good to me. Some examples of usage would be good.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Jan 27, 2017

Yes, those should be part of the tutorial. I'd actually like to suggest to broaden the scope of that tutorial to discuss Dimensions in general, with a significant section devoted to name and label but also covering the redim method and the difference between range and soft_range.

Another thing I've done in this PR is using strict dimension lookups in user facing APIs, which should result in much better tracebacks when referencing non-existent dimensions in many cases.

@philippjfr philippjfr changed the title Dimension aliases switched Add Dimension.label replacing global alias state Jan 27, 2017

@jlstevens

This comment has been minimized.

Copy link
Contributor

jlstevens commented Jan 27, 2017

Thanks @philippjfr!

I'll shortly write up a notebook tutorial describing aliases, redim and the distinction betweenrange soft_range and add it to this PR.

@jlstevens

This comment has been minimized.

Copy link
Contributor

jlstevens commented Jan 30, 2017

I now have part of a Dimensions tutorial but I would like to work on #843 before continuing work on it. I am happy with this PR and would like to merge it now. Are you ok with me adding the Dimensions notebook to a different PR?

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Jan 30, 2017

Sure, let's see how it goes. Better to get it in early to find issues.

@jlstevens jlstevens merged commit bfdad59 into master Jan 30, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 77.6%
Details
s3-reference-data-cache Tests passing no test data changes required.
Details

@philippjfr philippjfr deleted the dimension_aliases_switched branch Feb 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.