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

BUG Don't calculate levels if we aren't expanding #39

Conversation

stephen-hoover
Copy link
Contributor

If a user has excluded a column because they know that column has too many levels, they shouldn't then get a warning about that column. Make sure we don't count the levels in columns which are going to be dropped anyway. This fix also makes the _cols_to_expand attribute make more sense -- it no longer contains columns which we aren't going to expand.

@stephen-hoover stephen-hoover added this to the Next Version milestone May 16, 2018
If a user has excluded a column because they know that column has too many levels, they shouldn't then get a warning about that column. Make sure we don't count the levels in columns which are going to be dropped anyway. This fix also makes the `_cols_to_expand` attribute make more sense -- it no longer contains columns which we aren't going to expand.
@beckermr
Copy link
Contributor

I noticed this but I had not had time to report the bug. Thanks for the fix!

Copy link
Contributor

@elsander elsander left a comment

Choose a reason for hiding this comment

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

LGTM!

@elsander elsander assigned stephen-hoover and unassigned elsander May 17, 2018
@stephen-hoover stephen-hoover merged commit 6b92016 into civisanalytics:master May 17, 2018
@stephen-hoover stephen-hoover deleted the paro-663-only-warn-when-keeping-cols branch May 17, 2018 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants