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

keep extract and rename logic if using return_datasets #159

Merged
merged 2 commits into from
Feb 9, 2025

Conversation

alex-golts
Copy link
Collaborator

No description provided.

columns_to_extract=pairs_columns_to_extract,
rename_columns=pairs_rename_columns,
columns_to_extract=None,
rename_columns=None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about the columns_to_extract part,
it's possibly better to still keep using it in the op to make sure it's not too "heavy" per sample.
@mosheraboh do you think it matters or not ?

I think that you wanted mainly the rename to happen (also) in the dataframe.

Any option that you and @alex-golts agree on is fine by me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it (pairs_columns_to_extract ) set to None before? If not, I suggest keeping it.

Copy link
Collaborator

@YoelShoshan YoelShoshan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, comment inline

@alex-golts alex-golts merged commit 502d418 into main Feb 9, 2025
5 checks passed
@alex-golts alex-golts deleted the alex/keep_extract_and_rename_logic branch February 9, 2025 12:21
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.

3 participants