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

test refactor for floating point failures + mypy fix #1169

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

armaan-dhillon
Copy link
Contributor

@armaan-dhillon armaan-dhillon commented Feb 5, 2025

PR Description

  • refactors test_categorical_diff and test_diff_categorical_chi2_test which were failing locally due to some floating point errors but not failing when run on a different machine -- this refactors them so the floating point comparisons will be machine/platform agnostic
  • updates the flake8 rev in .pre-commit-config.yaml (was running into a local error with older version)
  • fixes one mypy error in dataprofiler/data_readers/parquet_data.py
  • adds inline # type: ignore[override] comments for 4 lines causing mypy override errors with TODO statements for context
  • above changes allow us to remove disable_error_code = override from setup.cfg

The 4 mypy override errors that this PR was supposed to address require adding a @property tag to the profile() method of NumericStatsMixin and this is a breaking change as it led to many tests failing. TODO comments highlight this.

@armaan-dhillon armaan-dhillon requested a review from a team as a code owner February 5, 2025 17:21
thebrianbn
thebrianbn previously approved these changes Feb 5, 2025
Copy link

@thebrianbn thebrianbn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@thebrianbn thebrianbn left a comment

Choose a reason for hiding this comment

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

LGTM

@armaan-dhillon armaan-dhillon merged commit 96febab into dev Feb 6, 2025
6 checks passed
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