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

Support single on option in Dataframe.join #865

Closed
pcapel opened this issue Feb 21, 2024 · 1 comment · Fixed by #866
Closed

Support single on option in Dataframe.join #865

pcapel opened this issue Feb 21, 2024 · 1 comment · Fixed by #866

Comments

@pcapel
Copy link
Contributor

pcapel commented Feb 21, 2024

I'm curious what you're thoughts are on:

  1. Allowing a single value as valid (as opposed to a list) in join (e.g. DF.join(df, on: :some_column) becomes valid)
  2. Giving an argument error when a list isn't given.

I was quickly moving through the docs and didn't notice the given examples all used a list of column names. The exception that is raised here is not very intuitive:

** (CaseClauseError) no case clause matching: {:fips, :inner}
    (explorer 0.8.0) lib/explorer/data_frame.ex:4931: Explorer.DataFrame.join/3
    #cell:rr454i5svjo3vvcp:1: (file)

Where in this case :fips is the column I would like to use. It seems like updating the case could work. Alternatively the case could default to an error that explains the issue more clearly.

In my opinion, allowing a single column value seems like the reasonable behavior. Happy to write up a PR for either case.

@josevalim
Copy link
Member

Good idea. We should wrap the on option with List.wrap :)

pcapel added a commit to pcapel/explorer that referenced this issue Feb 21, 2024
philss pushed a commit that referenced this issue Feb 21, 2024
* add failing test for case

#865

* use List.wrap/1 with opts[:on] for join/3

* update docs to reflect changes

---------

Co-authored-by: Philip Capel <pcapel@users.noreply.github.com>
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 a pull request may close this issue.

2 participants