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: merge fails when merging on column which contains values containing underscores #168

Closed
jpjagt opened this issue Apr 20, 2021 · 4 comments
Assignees

Comments

@jpjagt
Copy link
Contributor

jpjagt commented Apr 20, 2021

Describe the bug
danfojs-browser/src/core/merge.js stores sets of values by joining them with an underscore, and later splitting on underscore. however, when the values themselves contain underscores, this becomes problematic, as this split causes nonsensical columns to be defined when indexing the data.

To Reproduce
Steps to reproduce the behavior:

  1. create two dataframes, both with the same column consisting of values which contain underscores.
  2. attempt to dfd.merge those dataframes.
  3. get an error: Column length mismatch. You provided a column of length 1 but data has length of 3 (e.g.)

Expected behavior
the two dataframes should be merged without any trouble.

Desktop (please complete the following information):

  • OS: macOS, but any OS
  • Browser any browser
  • Version 0.2.5
@jpjagt
Copy link
Contributor Author

jpjagt commented Apr 20, 2021

i have submitted a PR which fixes this bug for both browser and node: #167

please tell me what you think!

thank you for your awesome work on this package! ❤️

@jpjagt
Copy link
Contributor Author

jpjagt commented Apr 20, 2021

by the way, i noticed the following changes in my git status:

modified:   danfojs-browser/lib/bundle.js
modified:   danfojs-browser/lib/bundle.js.LICENSE.txt
modified:   danfojs-browser/lib/bundle.js.map

i did not add this to the PR, because i did not know if i'm supposed to. but if i should add those changes, do let me know!

@risenW
Copy link
Member

risenW commented Apr 21, 2021

i have submitted a PR which fixes this bug for both browser and node: #167

please tell me what you think!

thank you for your awesome work on this package! ❤️

Thanks for taking out time to fix this! 😃 I'll certainly review and get this merge ASAP.

@risenW
Copy link
Member

risenW commented Apr 21, 2021

by the way, i noticed the following changes in my git status:

modified:   danfojs-browser/lib/bundle.js
modified:   danfojs-browser/lib/bundle.js.LICENSE.txt
modified:   danfojs-browser/lib/bundle.js.map

i did not add this to the PR, because i did not know if i'm supposed to. but if i should add those changes, do let me know!

Well not really, we committed the lib builds earlier when we were supporting install via Github. We don't do that anymore, so I'll add that to the git ignore file.

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

No branches or pull requests

2 participants