-
Notifications
You must be signed in to change notification settings - Fork 27
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
Automatic columns selector for joins function #168
base: master
Are you sure you want to change the base?
Conversation
Find out common columns between the 2 datasets and use the common columns in them as column-selector This change introduces a new arity in left-join function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to change api.clj
file since it's generated. https://github.com/scicloj/tablecloth/blob/master/src/tablecloth/api/api_template.clj#L182
@@ -53,22 +53,35 @@ | |||
(impl [(first cols-left) (first cols-right)] ds-left ds-right (or options {})) | |||
(multi-join impl ds-left ds-right cols-left cols-right options)))) | |||
|
|||
(defn- automatic-columns-selector [ds-left ds-right] | |||
(let [cols-l (set (column-names ds-left :all)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:all
can be skipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done,thanks.
@@ -53,22 +53,35 @@ | |||
(impl [(first cols-left) (first cols-right)] ds-left ds-right (or options {})) | |||
(multi-join impl ds-left ds-right cols-left cols-right options)))) | |||
|
|||
(defn- automatic-columns-selector [ds-left ds-right] | |||
(let [cols-l (set (column-names ds-left :all)) | |||
cols-r (set (column-names ds-right :all))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:all
can be skipped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, thanks
a32c8af
to
6702403
Compare
thanks, should I revert the changes in that ns completely? or its ok to leave what changes are there? |
very useful, I was thinking about requesting this as well,as such common case. |
This changes introduces an automatic column selector as discussed in #133 .
All the common columns in two datasets are used as the column selectors.