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

Use "default" instead of None when creating distilled_hierarchies. #306

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ltaylor-digmap
Copy link

Also, miscellaneous corrections in messages and documentation.
This solves issue #304 (provided this complies with the intention for the design of Cubes; that "default" hierarchy should be created if there is no hierarchy entry).

'namespace' misspelled as nampsace, corrected. Other misspellings in comments corrected
Also, miscellaneous corrections in messages and documentation.
@ltaylor-digmap
Copy link
Author

I made another commit which could fix the problem. i don't know how to merge the two commits, or make a new pull request which includes both changes.

Update workspace.py
'namespace' misspelled as nampsace, corrected. Other misspellings in comments corrected
@jjmontesl
Copy link
Contributor

Regarding your last comment, when you add a commit to a pull request branch the commit gets added to the pull request. That's ok.

@jjmontesl jjmontesl self-assigned this Jun 2, 2016
@jjmontesl jjmontesl added this to the 1.1 - Back to Basics, Back to SQL milestone Jun 2, 2016
@jjmontesl
Copy link
Contributor

I cannot reproduce #304 on current master.

I have tested this pull request and it causes several tests to fail. Seems there's other code referring to default dimensions as None.

An implicit hierarchy will always exist. I am unsure however whether a "default" hierarchy should be created or the extent of the impact of this patch.

I'd like to understand this better: what was the motivation to write this patch? I have no trouble accessing dimension or hierarchies without it.

@Stiivi Stiivi modified the milestones: 1.1.1, 1.1 Jun 3, 2016
@@ -957,7 +957,7 @@ def conditions_for_cuts(self, cuts):
conditions = []

for cut in cuts:
hierarchy = str(cut.hierarchy) if cut.hierarchy else None
hierarchy = str(cut.hierarchy) if cut.hierarchy else "default"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, we need this fix in master.

And the few lines below, in case of SetCut, we need to use "hierarchy" var instead of "str(cut.hierarchy)".
Here: https://github.com/DataBrewery/cubes/blob/master/cubes/sql/query.py#L973

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've fixed it here: #391

@Stiivi Stiivi self-assigned this Mar 14, 2017
@Stiivi Stiivi added the bug label Mar 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants