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

Refactor date.py #213

Closed
limlam96 opened this issue Apr 3, 2024 · 2 comments · Fixed by #246
Closed

Refactor date.py #213

limlam96 opened this issue Apr 3, 2024 · 2 comments · Fixed by #246
Assignees
Labels
help wanted Extra attention is needed

Comments

@limlam96
Copy link
Contributor

limlam96 commented Apr 3, 2024

What?
This collection of transformers needs a bit of a rework before setting up the new testing framework, the classes are set up inconsistently in a way that does not support the new approach.

Suggested actions:

  • BaseDateTransformer to implement transform method, that calls check_columns_are_date_or_datetime
  • Deprecate cast_columns_to_datetime from BaseDateTransformer, throw error directing to existing ToDatetimeTransformer if columns fail type checks
  • Some complications are introduced by transformers checking for datetime OR date type, which allows the possibility of mismatched types across manipulated columns. Suggest we make transformers more opinionated, introduce a type argument that allows date or datetime and checks accordingly. This would allow us to deprecate '_cast_non_matching_columns', 'match_column_datatypes'

Other thoughts:

  • We have DateDiffLeapYearTransformer and DateDifferenceTransformer, can these be condensed?
@davidhopkinson26
Copy link
Contributor

Consolidating DateDiffLeapYearTransformer and DateDifferenceTransformer now has its own issue in #244

@limlam96
Copy link
Contributor Author

Closed with this PR #246

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants