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 tests for duplicate additional_variables/copy_variables in normal… #348

Merged

Conversation

georgewambold
Copy link
Contributor

…ize_entity

@CLAassistant
Copy link

CLAassistant commented Dec 13, 2018

CLA assistant check
All committers have signed the CLA.

@kmax12
Copy link
Contributor

kmax12 commented Dec 13, 2018

even though this is caught now, it is sort of just an fortunate "accident" that we now check for duplicate columns later in the process of loading an entity.

I'd say it is still worth it to add a check in normalize_entity(), so that we can give a more specific error of "Duplicate columns in copy_variables" or "Duplicate columns in additional_variables" when the user does this.

Want to add that? the test cases look good though

@georgewambold
Copy link
Contributor Author

Ah, ok-- Yeah I'm down to add the more specific errors.

@georgewambold georgewambold force-pushed the add_duplicate_columns_custom_error branch from 5b01742 to 4414ffe Compare December 14, 2018 15:40
@kmax12
Copy link
Contributor

kmax12 commented Dec 14, 2018

Looks good, merging. Thanks for the contribution, we appreciate it!

@kmax12 kmax12 merged commit d0ebff1 into alteryx:master Dec 14, 2018
jeff-hernandez pushed a commit to jeff-hernandez/featuretools that referenced this pull request Dec 14, 2018
@rwedge rwedge mentioned this pull request Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants