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

Pass model_class to schema_extra staticmethod #1125

Merged
merged 8 commits into from Jan 6, 2020

Conversation

therefromhere
Copy link
Contributor

@therefromhere therefromhere commented Dec 23, 2019

Change Summary

Pass model_class to schema_extra staticmethod.

Related issue number

Resolves #1122

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)

@codecov
Copy link

codecov bot commented Dec 23, 2019

Codecov Report

Merging #1125 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1125   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          20      20           
  Lines        3445    3450    +5     
  Branches      665     667    +2     
======================================
+ Hits         3445    3450    +5
Impacted Files Coverage Δ
pydantic/schema.py 100% <100%> (ø) ⬆️

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 e169bd6...e740ad7. Read the comment docs.

@therefromhere
Copy link
Contributor Author

therefromhere commented Dec 23, 2019

@samuelcolvin I think this is ready for review now, the tests seem to be failing due to an unrelated issue?

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.

sorry for the delay in getting back to this.

changes/1125-therefromhere.md Outdated Show resolved Hide resolved
docs/examples/schema_extra_callable.py Outdated Show resolved Hide resolved
docs/examples/schema_extra_callable.py Outdated Show resolved Hide resolved
docs/examples/schema_extra_callable.py Outdated Show resolved Hide resolved
pydantic/schema.py Outdated Show resolved Hide resolved
tests/test_schema.py Show resolved Hide resolved
@therefromhere
Copy link
Contributor Author

therefromhere commented Jan 2, 2020 via email

@therefromhere
Copy link
Contributor Author

@samuelcolvin I've made those changes + rebased.

@samuelcolvin samuelcolvin merged commit cd8b504 into pydantic:master Jan 6, 2020
@samuelcolvin
Copy link
Member

great, thanks.

andreshndz pushed a commit to cuenca-mx/pydantic that referenced this pull request Jan 17, 2020
* Pass model_class to schema_extra staticmethod

Resolves pydantic#1122

* Add changelog

* Apply suggestions from code review

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

* Fix import after rebase

* Fix test bug

* Use TypeError instead of assert as per review

* Rename var so declaration fits one one line

* tiny tweaks

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>
@andriilahuta
Copy link
Contributor

@therefromhere @samuelcolvin any particular reason why schema_extra is not allowed to be a classmethod?

Also, when schema_extra is a regular method - TypeError is not raised.

@therefromhere
Copy link
Contributor Author

I think because it would be confusing - bear in mind it'll be a classmethod of Config (so cls wouldn't give access to your model class). This threw me originally when I was talking about it.

What would the use case for a classmethod be here?

Good point about regular methods not hitting TypeError, that's a bug.

@andriilahuta
Copy link
Contributor

I'm using classmethods for config inheritance, so things break for me when updating from v1.3.

alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support passing model class to schema_extra?
3 participants