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

Add GetterDict example #2463

Merged
merged 2 commits into from
May 12, 2021
Merged

Add GetterDict example #2463

merged 2 commits into from
May 12, 2021

Conversation

nuno-andre
Copy link
Contributor

@nuno-andre nuno-andre commented Mar 3, 2021

Here is a very simple example of how to subclass GetterDict to process a class with no mapping protocol while dealing with the impedance mismatch between models (DOM's attributes and children vs OOP's properties).

I have added three optional fields to show "missing attribute", "missing children", and "missing children attribute".

closes #1745

@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #2463 (cb0f115) into master (429b439) will not change coverage.
The diff coverage is n/a.

❗ Current head cb0f115 differs from pull request most recent head 010ce73. Consider uploading reports for the commit 010ce73 to get more accurate results

@@            Coverage Diff            @@
##            master     #2463   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         5099      5100    +1     
  Branches      1048      1048           
=========================================
+ Hits          5099      5100    +1     
Impacted Files Coverage Δ
pydantic/main.py 100.00% <0.00%> (ø)
pydantic/generics.py 100.00% <0.00%> (ø)

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 429b439...010ce73. Read the comment docs.

@nuno-andre
Copy link
Contributor Author

I've also documented the _missing sentinel behavior. But I'm not quite sure that's the intended way to raise missing field errors, as the GetterDict code seems to suggest that KeyError should do so (which would make a lot of sense IMO). However, this is not the case.

Could you please confirm this assumption?

@PrettyWood
Copy link
Collaborator

@nuno-andre Thanks a lot for this doc, it was needed!
And good remark for the KeyError thing, which would indeed make sense.
But for now better stick with what you did and maybe open a discussion to suggest a refacto?

@nuno-andre
Copy link
Contributor Author

nuno-andre commented Mar 18, 2021

Of course, just wanted to confirm that it wasn't an unintended behavior or a regression.

@PrettyWood
Copy link
Collaborator

@nuno-andre can you add a change file please? :)

@PrettyWood PrettyWood added awaiting author revision awaiting changes from the PR author and removed ready for review labels Apr 5, 2021
@nuno-andre
Copy link
Contributor Author

Done! I've just realized I'd completely ignored the PR template. Sorry for that.

@PrettyWood PrettyWood added ready for review and removed awaiting author revision awaiting changes from the PR author labels Apr 6, 2021
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 looks good.

docs/usage/models.md Outdated Show resolved Hide resolved
@samuelcolvin
Copy link
Member

please update, otherwise I'll accept that one tiny change some time soon and merge.

@github-actions github-actions bot added awaiting author revision awaiting changes from the PR author and removed ready for review labels May 9, 2021
Co-authored-by: Samuel Colvin <samcolvin@gmail.com>
@nuno-andre
Copy link
Contributor Author

I guess this CI error isn't related to the PR, right?

@samuelcolvin
Copy link
Member

no, that was just a problem with that CI script.

@samuelcolvin samuelcolvin merged commit b718e8e into pydantic:master May 12, 2021
@nuno-andre nuno-andre deleted the master branch May 12, 2021 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting author revision awaiting changes from the PR author documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document GetterDict for ORM mode properly properly
3 participants