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

[ENH] MergeData: Don't remove duplicate columns with different data #4100

Merged
merged 2 commits into from
Oct 29, 2019

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Oct 11, 2019

Fixes #4077.

Includes
  • Code changes
  • Tests
  • Documentation

@janezd janezd force-pushed the merge-data-non-duplicate branch from 14b22c6 to e8e672f Compare October 11, 2019 19:00
@codecov
Copy link

codecov bot commented Oct 11, 2019

Codecov Report

Merging #4100 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4100      +/-   ##
==========================================
+ Coverage   85.68%   85.69%   +0.01%     
==========================================
  Files         390      390              
  Lines       69732    69796      +64     
==========================================
+ Hits        59750    59812      +62     
- Misses       9982     9984       +2

@janezd janezd force-pushed the merge-data-non-duplicate branch 2 times, most recently from 549eed9 to 3f935f9 Compare October 23, 2019 14:46
@VesnaT
Copy link
Contributor

VesnaT commented Oct 25, 2019

There is a bug. Could had been there before the PR, but can you please check anyway.
To reproduce:

Screenshot 2019-10-25 at 11 09 54

The scariest thing here is, all the tests passed.

@janezd janezd force-pushed the merge-data-non-duplicate branch from 3f935f9 to 3d0787a Compare October 28, 2019 20:56
vstack of Y failed when all output rows were matched and the output had a single class variable.
i
@janezd
Copy link
Contributor Author

janezd commented Oct 28, 2019

Interesting.

Yes, it was unrelated to changes in this PR. I fixed it and wrote a test that would fail without the fix.

@VesnaT VesnaT merged commit 294b398 into biolab:master Oct 29, 2019
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.

Merge Data merges columns I don't want merged
2 participants