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

Adding type hints #116 #128

Merged
merged 62 commits into from
Dec 29, 2023
Merged

Conversation

Sarah-TaylorKnight
Copy link
Contributor

Working through the repo to add in type hints across the code base in order to address ANN ruff rules:

ANN001: missing-type-function-argument - Missing type annotation for function argument
ANN002: missing-type-args - Missing type annotation for *{name}
ANN003: missing-type-kwargs - Missing type annotation for **{name}
ANN101: missing-type-self - Missing type annotation for {name} in method
ANN102: missing-type-cls - Missing type annotation for {name} in classmethod
ANN201: missing-return-type-undocumented-public-function - Missing return type annotation for public function {name}
ANN202: missing-return-type-private-function - Missing return type annotation for private function {name}
ANN204: missing-return-type-special-method - Missing return type annotation for special method {name}
ANN205: missing-return-type-static-method - Missing return type annotation for staticmethod {name}
ANN206: missing-return-type-class-method - Missing return type annotation for classmethod {name}
ANN401: any-type - Dynamically typed expressions (typing.Any) are disallowed in {name}

Notes:
I'd suggest we exclude ANN101 as it adds clutter without much information, and as ruff put it "Note that many type checkers will infer the type of self automatically, so this annotation is not strictly necessary."

richardangell and others added 22 commits October 5, 2021 13:59
Develop into master (0.2.15)
Bring v0.3.1 changes into main
Bring DSF notebook into main
Interaction transformer (#38)
-added support for prior mean encoding (regularised encodings)
-added support for weights to mean, median and mode imputers
-added classname() method to BaseTransformer and prefixed all errors with classname call for easier debugging
-added DatetimeInfoExtractor transformer
-added DatetimeSinusoidCalculator
-added TwoColumnOperatorTransformer
-added StringConcatenator
-added SetColumnDtype
-added waring to MappingTransformer for unexpected changes in dtype
-added new module tubular/comparison.py containing EqualityChecker
-added PCATransformer
-updated black version to 22.3.0 and flake8 version to 5.0.4 to fix compatibility issues
-removed kwargs argument from BaseTransfomer
-updated version to 0.3.3
-updated changelog and docs

Co-authored-by: Claire_Fromholz <[email protected]>
Co-authored-by: Fromholz <[email protected]>
Revert "v0.3.3 changes into main"
0.3.4 changes into main (also merges changes for 0.3.3 after reversion)
merge changes for 0.3.5
Merge changes for 0.3.6
Update _version.py to 0.3.6
0.3.7 changes into main
Version update into main
0.3.8 changes into main
@Sarah-TaylorKnight
Copy link
Contributor Author

Looking at ANN03 - type hinting kwargs - this is quite difficult to implement in general, and tubular in general does not have many params passed through as kwargs - unsure how to proceed with these

@davidhopkinson26 davidhopkinson26 marked this pull request as ready for review September 12, 2023 11:32
tubular/capping.py Show resolved Hide resolved
tubular/capping.py Outdated Show resolved Hide resolved
tubular/comparison.py Outdated Show resolved Hide resolved
tubular/dates.py Outdated Show resolved Hide resolved
tubular/dates.py Outdated Show resolved Hide resolved
tubular/nominal.py Show resolved Hide resolved
tubular/numeric.py Outdated Show resolved Hide resolved
tubular/numeric.py Outdated Show resolved Hide resolved
tubular/numeric.py Outdated Show resolved Hide resolved
tubular/strings.py Outdated Show resolved Hide resolved
@adamsardar
Copy link
Contributor

adamsardar commented Dec 29, 2023

  • Comments above have been incorporated and resolved
  • main has been merged and build restored to passing
  • PR ready to merge now
  • three responses to comments above outstanding All resolved

A really solid bit of work here - thanks for the hard work all!

tubular/dates.py Outdated Show resolved Hide resolved
Copy link
Contributor

@adamsardar adamsardar left a comment

Choose a reason for hiding this comment

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

Looking good!

I'm going to approve and merge because:

  • I've just swooped in at the end to get it over the line
  • Sarah wrote this, Tommy has reviewed, Dave has commented. I think that it's had all the eyeballs that it's going to get!
  • Coffee is for (branch) closers. Always Be Closing.

@adamsardar adamsardar merged commit df6188f into develop Dec 29, 2023
1 check passed
@davidhopkinson26 davidhopkinson26 deleted the feature/116-ann-flake8-annotations branch March 21, 2024 09:20
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.

5 participants