-
-
Notifications
You must be signed in to change notification settings - Fork 363
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
Datetime display options #885
Conversation
… the javascript date format is correct for a display option Add Date time display options serializer Add validation for date time display option serializer Fix Column serializer and column reflection to use plain_type as `DISPLAY_OPTIONS_SERIALIZER_MAPPING_KEY` for display options serializer mapping instead of sql alchemy type
Blocked by #865 |
Codecov Report
@@ Coverage Diff @@
## master #885 +/- ##
==========================================
+ Coverage 93.20% 93.29% +0.08%
==========================================
Files 86 86
Lines 3104 3160 +56
==========================================
+ Hits 2893 2948 +55
- Misses 211 212 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Overall, this looks pretty good. Some comments:
- The validation is spread over two functions called in different ways, which is a bit confusing. Couldn't one call the other?
- One of the validations was missing a piece (I think; see the specific comment)
- Please make
vulture
happy so the pipeline passes.
…ect instead of the timestamp part
…atetime-display-options
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.
Looks great to me; thanks!
Fixes #428
Accept display options for datetime column types.
Technical details
This PR adds a serializer to accept and validate datetime column
display_options
. It uses arrow library to validateformat
field values to accept only valid datetime object and restrict the format string based on types(date column can't accept format string with time formats).Checklist
Update index.md
).master
branch of the repositoryvisible errors.
Developer Certificate of Origin
Developer Certificate of Origin