-
-
Notifications
You must be signed in to change notification settings - Fork 782
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
refactored DateWidget & DateTimeWidget & TimeWidget #1839
refactored DateWidget & DateTimeWidget & TimeWidget #1839
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting.
- I had some test failures when running this, please can you look into it.
- Please can you run pre-commit to format your code.
- Feel free to add your name to AUTHORS
- Make BaseDateTimeWidget class abstract to enforce its use as a base class and prevent direct instantiation. - Run pre-commit hooks to ensure code formatting aligns with project standards. - Add name to AUTHORS list as a new contributor to the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still have test failures, can you run make test
locally to get them to pass?
I pushed a change to |
I also refactored to use mixin instead of abstract base class. I think this is cleaner because the date parsing logic is now isolated to the widgets module, and the abstract declaration had no effect (it was still possible to instantiate the base class). We don't want the |
Thanks for all your help with this. |
Problem
Closes #1835
The existing date handling widget classes (DateWidget, DateTimeWidget, and TimeWidget) had redundant code for format initialization and parsing, making the codebase hard to maintain.
Solution
I introduced a BaseDateTimeWidget class to centralize common format handling and parsing logic. DateWidget, DateTimeWidget, and TimeWidget now inherit from this base class, reducing redundancy and streamlining operations.