8000 Add datetime field to default_factory example by StephenBrown2 · Pull Request #1301 · pydantic/pydantic · GitHub 8000
[go: up one dir, main page]
Skip to content

Add datetime field to default_factory example #1301

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.

Sign up for GitHub

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

Jump to bottom
Merged
samuelcolvin merged 4 commits into from
Mar 18, 2020
Merged

Add datetime field to default_factory example #1301

samuelcolvin merged 4 commits into from
Mar 18, 2020

Conversation

@StephenBrown2
Copy link
Contributor
@StephenBrown2 StephenBrown2 commented Mar 11, 2020 •
edited
Loading

Change Summary

While looking through the docs, I was happy to find that default_factory will be added to 1.5 provisionally, however, I was specifically searching for an example where datetime was being used, and my searches came up fruitless until I checked the closed issues. This adds an example for searchability purposes (and works as expected in my tests)

Related issue number

None

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020

Updated page: https://pydantic.helpmanual.io/dev/pr-1301/usage/models/#field-with-dynamic-default-value

@codecov
Copy link
codecov bot commented Mar 11, 2020 •
edited
Loading

Codecov Report

Merging #1301 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1301   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files          21       21           
  Lines        3713     3713           
  Branches      731      731           
=======================================
  Hits         3709     3709           
  Misses          2        2           
  Partials        2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcf5478...b23ffe5. Read the comment docs.

samuelcolvin
samuelcolvin reviewed Mar 11, 2020
View reviewed changes
Copy link
Member
@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020

Changed the asserts to prints, left the != in the string to show them on the same line in a self-explanatory manner. Let me know if you'd prefer them on separate lines or otherwise.

@samuelcolvin
Copy link
Member
samuelcolvin commented Mar 11, 2020

single quotes please 😄

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020

Removed bad quotes, but I noticed that it would have been checked if the lint step included the docs/ folder.

I did so and found that there are a bunch of lints (including other double quote usage) in the examples, shall I open another PR to fix those?

@samuelcolvin
Copy link
Member
samuelcolvin commented Mar 11, 2020

up to you, if it's easy you could fix it here, otherwise leave the linting unchanged, and I'll do it elsewhere.

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020

Fairly easy. I added a separate isort/black_docs command in the makefile, to apply an 80-character limit to docs only, matching most of the current formatting, and better for the view inside mkdocs.

I did them in separate commits for easy reversion if desired, and it looks like I needed to allow two newlines in the exec_examples script (Fabulous bit of code, that) to let it build properly.

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020 •
edited
Loading

Not sure why this is failing on 3.6 and 3.7, when it says python 3.8 only and has passed before...
https://github.com/samuelcolvin/pydantic/pull/1301/files#diff-747597a1ac9f51911f3d0b5243f3ea74R26

@StephenBrown2
Copy link
Contributor Author
StephenBrown2 commented Mar 11, 2020

Looks like it's a bit more tricky than I anticipated, so I'll revert it for now and leave it for later.

@samuelcolvin
Copy link
Member
samuelcolvin commented Mar 11, 2020

Great, I think the reason I didn't do linting was that it wasn't trivial. I'll fix soon.

@StephenBrown2 StephenBrown2 force-pushed the add_datetime_default_factory_example branch from 7abfc6e to 66bbb54 Compare March 15, 2020 19:27
StephenBrown2
StephenBrown2 commented Mar 15, 2020
View reviewed changes
StephenBrown2
StephenBrown2 commented Mar 15, 2020
View reviewed changes
@StephenBrown2
Add datetime field to default_factory example
2002d49 
Print instead of assert
Remove bad quotes
Remove bad quotes from all examples
@StephenBrown2
Add change file
af2bde9
@StephenBrown2 StephenBrown2 force-pushed the add_datetime_default_factory_example branch from 66bbb54 to af2bde9 Compare March 17, 2020 22:47
@samuelcolvin samuelcolvin merged commit 13b002e into pydantic:master Mar 18, 2020
@StephenBrown2 StephenBrown2 deleted the add_datetime_default_factory_example branch May 1, 2020 21:52
davidhewitt pushed a commit that referenced this pull request Oct 19, 2025
@sydney-runkle
Prep for 2.18.3 Release (#1301)
5228721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelcolvin samuelcolvin samuelcolvin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@StephenBrown2 @samuelcolvin
0