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

support ForwardRef in Python 3.6 #706

Merged

Conversation

koxudaxi
Copy link
Sponsor Contributor

@koxudaxi koxudaxi commented Aug 2, 2019

Change Summary

Support ForwardRef in Python 3.6.
The Feature is talked in the issue. #463
The PR supports features on this list in Python 3.6

  • _ForwardRef
  • resolve string annotations (eg. List['User'])
  • update_forward_refs

Related issue number

#463

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • HISTORY.rst has been updated
    • if this is the first change since a release, please add a new section
    • include the issue number or this pull request number #<number>
    • include your github username @<whomever>

@codecov
Copy link

@codecov codecov bot commented Aug 2, 2019

Codecov Report

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

@@          Coverage Diff          @@
##           master   #706   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2721   2725    +4     
  Branches      539    539           
=====================================
+ Hits         2721   2725    +4

Copy link
Owner

@samuelcolvin samuelcolvin left a comment

otherwise looks good to me.

@@ -0,0 +1,300 @@
"""
Copy link
Owner

@samuelcolvin samuelcolvin Aug 2, 2019

Choose a reason for hiding this comment

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

looks like much of this might be duplicates of the python 3.7 tests? couldn't we just remove the "skip" and run the same test for both? (At least in some cases).

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 2, 2019

Choose a reason for hiding this comment

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

Python 3.6 and pyhton3.7 are a little different (eg. _ForwardRef and ForwardRef).
I have to change exists test code.
I try to do it.

Copy link
Owner

@samuelcolvin samuelcolvin Aug 2, 2019

Choose a reason for hiding this comment

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

great, I think better to have one set of tests than two, even if it means a bit of repeated try:...except: or if sys.version...else.

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 2, 2019

Choose a reason for hiding this comment

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

I have changed test_py37.py to test_forward_ref.py and deleted test_py36.py

@StephenBrown2
Copy link
Collaborator

@StephenBrown2 StephenBrown2 commented Aug 2, 2019

Does this mean, to support Python 3.6 in my code, I will need to add

try:
    from typing import ForwardRef
except ImportError:
    from typing import _ForwardRef as ForwardRef

anywhere I have need of a forward ref? Or will self-referencing strings still work fine? This makes me think it will, so I'm hoping so.

e.g.

this currently works for me in Python 3.7:

from typing import List

from pydantic import BaseModel

class CalendarRule(BaseModel):
    id: int
    calendar: int
    children: List["CalendarRule"]
    ordering: int
    rule_attribute: str
    rule_value: str


CalendarRule.update_forward_refs()

but I will not be able to reference it by string value, and instead will need to do:

from typing import List

from pydantic import BaseModel

try:
    from typing import ForwardRef
except ImportError:
    from typing import _ForwardRef as ForwardRef

CalendarRule = ForwardRef("CalendarRule")

class CalendarRule(BaseModel):
    id: int
    calendar: int
    children: List[CalendarRule]
    ordering: int
    rule_attribute: str
    rule_value: str


CalendarRule.update_forward_refs()

@koxudaxi
Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi commented Aug 3, 2019

@StephenBrown2

Does this mean, to support Python 3.6 in my code, I will need to add

Yes, If you need ForwardRef in Python3.6 and python3.7

anywhere I have need of a forward ref? Or will self-referencing strings still work fine? This makes me think it will, so I'm hoping so.

No, you don't need forward ref. you can use self-referencing string in python3.6 and python3.7.

Also, you can install the PR version in your local machine to try it.
$ pip install git+https://github.com/koxudaxi/pydantic.git@support_forwardred_in_python36

def test_forward_ref_dataclass(create_module):
module = create_module(
"""
from __future__ import annotations
Copy link
Owner

@samuelcolvin samuelcolvin Aug 5, 2019

Choose a reason for hiding this comment

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

I think we still need a 3.7 only test with from __future__ import annotations to check that this works with future annotations.

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 5, 2019

Choose a reason for hiding this comment

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

I have added unittest for python3.7

from typing import List
from pydantic import BaseModel, Schema

class Account(BaseModel):
name: str
subaccounts: List[Account] = []
subaccounts: List['Account'] = []
Copy link
Owner

@samuelcolvin samuelcolvin Aug 5, 2019

Choose a reason for hiding this comment

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

again I'm afraid I think we need both this test and the 3.7 only test with from __future__ import annotations

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 5, 2019

Choose a reason for hiding this comment

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

I add unittest for python3.7

def test_circular_reference_json_schema(create_module):
module = create_module(
"""
from __future__ import annotations
Copy link
Owner

@samuelcolvin samuelcolvin Aug 5, 2019

Choose a reason for hiding this comment

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

and again.

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 5, 2019

Choose a reason for hiding this comment

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

and I add unittest for python3.7.

def test_self_forward_ref_collection(create_module):
module = create_module(
"""
from typing import ForwardRef, List, Dict
from typing import List, Dict
try:
Copy link
Owner

@samuelcolvin samuelcolvin Aug 5, 2019

Choose a reason for hiding this comment

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

might be good to add this to pydantic.utils.

Then you can do from pydantic.utils import ForwardRef in tests but also people can use that in their own code if they want to work with 3.6 and 3.7?

Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi Aug 5, 2019

Choose a reason for hiding this comment

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

OK, I add pydantic.utils.
They can use from pydantic.utils import ForwardRef with 3.6 and 3.7.

@koxudaxi
Copy link
Sponsor Contributor Author

@koxudaxi koxudaxi commented Aug 5, 2019

I have merged history :)

@samuelcolvin samuelcolvin merged commit 6be7c21 into samuelcolvin:master Aug 5, 2019
10 checks passed
@samuelcolvin
Copy link
Owner

@samuelcolvin samuelcolvin commented Aug 5, 2019

great, thank you very much.

@StephenBrown2
Copy link
Collaborator

@StephenBrown2 StephenBrown2 commented Aug 6, 2019

Woohoo! Looking forward to v0.32 now! Thanks very much for this.

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.

None yet

3 participants