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

dev: fix some mypy errors and switch to ruff #511

Merged
merged 22 commits into from
Nov 22, 2024
Merged

Conversation

jrycw
Copy link
Collaborator

@jrycw jrycw commented Nov 17, 2024

Hello team,

I ran mypy on our codebase and resolved around 60 reported errors in this PR (leaving 300+ errors still to address). Most of the changes involve refining type hints.

Based on my observations, mypy seems to struggle with the following scenarios:

  • Redefining a variable within the same scope while changing its type.
  • Fully understanding the logic behind our DataFrame backend.
  • Recognizing that GTSelf is a TypeVar for GTData, which causes many GT attributes to be flagged as "not found."
  • Handling returned values that might include None, which can be challenging to annotate correctly.

Feel free to accept, reject, or modify anything in this PR.

jrycw added 15 commits November 17, 2024 19:31
* Remove unused function signatures.
* Remove unused import modules.
* Clarify that `sides` in `CellStyleBorders` does not support `ColumnExpr`.
* Move `_flatten_list()` to `_utils.py`
* Use `_flatten_list()` to deduplicate formatted cells in `_migrate_unformatted_to_output()`.
* Refactor `ColumnAlignment` to a `TypeAlias` as the `Enum` definition is unused in the code.
* Propose preserving the signatures of `__new__()` and `__init__()` in `Boxhead`.
Copy link

codecov bot commented Nov 17, 2024

Codecov Report

Attention: Patch coverage is 90.90909% with 9 lines in your changes missing coverage. Please review.

Project coverage is 89.71%. Comparing base (538fbf1) to head (019f637).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
great_tables/_export.py 25.00% 3 Missing ⚠️
great_tables/_formats.py 50.00% 2 Missing ⚠️
great_tables/_body.py 50.00% 1 Missing ⚠️
great_tables/_gt_data.py 96.42% 1 Missing ⚠️
great_tables/_utils.py 96.66% 1 Missing ⚠️
great_tables/gt.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #511      +/-   ##
==========================================
+ Coverage   89.38%   89.71%   +0.32%     
==========================================
  Files          45       45              
  Lines        5239     5202      -37     
==========================================
- Hits         4683     4667      -16     
+ Misses        556      535      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@machow
Copy link
Collaborator

machow commented Nov 21, 2024

Thanks so much for stripping so much dead code and fixing types. We're working on completing a pyopensci review for Great Tables, which recommended linting for unused imports (pyOpenSci/software-submission#202 (comment)).

I think we disabled unused imports and unused variables from our linter, when we did a very early refactor. Rich and I are going to pair tomorrow to re-enable more linting rules, so we can stick to the level of quality in this PR 😅. If it's okay, we'll just add to this PR and then merge.

@jrycw
Copy link
Collaborator Author

jrycw commented Nov 22, 2024

Thanks so much for stripping so much dead code and fixing types. We're working on completing a pyopensci review for Great Tables, which recommended linting for unused imports (pyOpenSci/software-submission#202 (comment)).

I think we disabled unused imports and unused variables from our linter, when we did a very early refactor. Rich and I are going to pair tomorrow to re-enable more linting rules, so we can stick to the level of quality in this PR 😅. If it's okay, we'll just add to this PR and then merge.

Wow, that’s great! It’s exciting to see so many valuable suggestions for this project. Personally, I’d recommend we clean up the issues—for example, closing those that have already been addressed or answered.

@machow machow requested a review from rich-iannone November 22, 2024 21:01
Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

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

LGTM!

@machow machow changed the title Address type hinting and fix some mypy errors dev: fix some mypy errors and switch to ruff Nov 22, 2024
@machow machow merged commit 549f668 into posit-dev:main Nov 22, 2024
14 checks passed
@jrycw jrycw deleted the cleanup branch November 23, 2024 03:29
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