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

Add ignore_columns option to dataframe_comparer.py #141

Merged

Conversation

kasztp
Copy link
Contributor

@kasztp kasztp commented Oct 11, 2024

Add ignore_columns option to:
assert_df_equality()
assert_approx_df_equality()

Implementation for #140

PR Checklist

  • A description of the changes is added to the description of this PR.
  • If there is a related issue, make sure it is linked to this PR.
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added or modified a feature, documentation in docs is updated

Description of changes

Often when comparing dataframes we have some columns which can / should be ignored during the equality check. E.g. timestamp columns, or UUID.

With the implementation of ignore_columns option in:
assert_df_equality(),
and
assert_approx_df_equality(),
functions, users can now specify columns to be ignoered during databrame comparison.

Add ignore_columns option to:
assert_df_equality()
assert_approx_df_equality()
Copy link
Collaborator

@SemyonSinchenko SemyonSinchenko left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution!

@SemyonSinchenko
Copy link
Collaborator

SemyonSinchenko commented Oct 12, 2024

@kasztp Can you also run a pre-commit with ruff to fix failing code-style check?
https://github.com/MrPowers/chispa/actions/runs/11291104694/job/31447515682?pr=141

@kasztp
Copy link
Contributor Author

kasztp commented Oct 12, 2024

@SemyonSinchenko Done, thanks for spotting!

@kasztp
Copy link
Contributor Author

kasztp commented Oct 24, 2024

@SemyonSinchenko Anything else that needs to be done form my side to get this merged? :)

@SemyonSinchenko
Copy link
Collaborator

Sorry, I lost it

@SemyonSinchenko SemyonSinchenko merged commit 202ab2a into MrPowers:main Oct 24, 2024
15 checks passed
@kasztp
Copy link
Contributor Author

kasztp commented Oct 25, 2024

Thanks for the review!

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.

2 participants