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

Inner join handle common columns #73

Merged
merged 6 commits into from
Oct 13, 2024
Merged

Inner join handle common columns #73

merged 6 commits into from
Oct 13, 2024

Conversation

Vindaar
Copy link
Member

@Vindaar Vindaar commented Oct 13, 2024

- *Potentially BREAKING behavior*: ~innerJoin~ now has another
  parameter ~commonColumns~, which handles how columns that are common
  to both dataframes are handled.
  Previously, we wrongly assumed that the data in all common columns
  must match exactly. In that case it didn't matter which column to
  take.
  However, if they did *not* match, the data for the common columns
  (including the one we joined by) was corrupted and left at default
  initialization from the first mismatch in a common column.
  There are now 3 different ways:
  - ~ccLeft~: keep the data of the left input
  - ~ccRename~: Rename the common columns to ~*_left~ and ~*_right~
    (default)
  - ~ccDrop~: Drop common columns that are not the joined one.
  We choose to use ~ccRename~ as the default, because it keeps all
  information. It is a breaking change however, because the common
  columns now have a different name. But imo it's better here to make
  people aware that this change happened instead of giving them wrong
  data.
  Feel free to conflict me on this.

@Vindaar
Copy link
Member Author

Vindaar commented Oct 13, 2024

Merging despite CI failures. Those are unrelated (due to nimcuda) and will be fixed separately.

@Vindaar Vindaar merged commit d364273 into master Oct 13, 2024
4 of 9 checks passed
narimiran pushed a commit to nim-lang/Nim that referenced this pull request Jan 14, 2025
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.

1 participant