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

Added __get_pydantic_json_schema__ method with format='tel' #106

Merged
merged 6 commits into from
Nov 11, 2023
Merged

Added __get_pydantic_json_schema__ method with format='tel' #106

merged 6 commits into from
Nov 11, 2023

Conversation

hasansezertasan
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a973b79) 100.00% compared to head (ca9fc00) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #106   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          637       642    +5     
  Branches       158       159    +1     
=========================================
+ Hits           637       642    +5     
Files Coverage Δ
pydantic_extra_types/phone_numbers.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Where did you get this format from?

@hasansezertasan
Copy link
Contributor Author

hasansezertasan commented Nov 9, 2023

Where did you get this format from?

I was playing with json-editor/json-editor and they use this format.

I wasn't sure about opening this pull request so before I checked JSON Schema but couldn't find anything and then I checked Alpaca Forms which uses phone as the format. I was confused about using tel as a format, in the end I checked the html attributes, phone number inputs uses tel, here the example: HTML input type="tel".

Edit: After you asked I checked JSON Schema again and found this page: A JSON Media Type for Describing the Structure and Meaning of JSON Documents. In the 5.19. format section I saw this line: phone - This should be a phone number (format may follow E.123). I think I should change format='tel' to format='phone'

What do you think @Kludex

@Kludex
Copy link
Member

Kludex commented Nov 9, 2023

The document you are using to "prove" it should be "phone" is expired by 13 years already... Do you have a better source?

@hasansezertasan
Copy link
Contributor Author

The document you are using to "prove" it should be "phone" is expired by 13 years already... Do you have a better source?

I believe not, I'll do more digging. Until then, I'm converting this one to draft.

@hasansezertasan hasansezertasan marked this pull request as draft November 9, 2023 10:23
@hasansezertasan
Copy link
Contributor Author

hasansezertasan commented Nov 11, 2023

@Kludex 👋 Hope you're doing well.

I was searching for a better source to implement a valid schema that includes format for PhoneNumber field.

The document you are using to "prove" it should be "phone" is expired by 13 years already... Do you have a better source?

You're right about that. But thanks to you I saw this line in the draft-04 (Expires: August 26, 2017)

"format": remove attributes "phone", "style", "color"; rename "ip-address" to "ipv4"; add references for all attributes.

Hance pydantic-extra-types uses format='color' in Color field, I think using format='phone' in the PhoneNumber field seems allright.

What do you think we should do? Remove format='color' from the Color field which is expired by 13 years already... or add format='phone' to PhoneNumber field?

@Kludex
Copy link
Member

Kludex commented Nov 11, 2023

You're right about that. But thanks to you I saw this line in the draft-04 (Expires: August 26, 2017)

Link?

@hasansezertasan
Copy link
Contributor Author

hasansezertasan commented Nov 11, 2023

"format": remove attributes "phone", "style", "color"; rename "ip-address" to "ipv4"; add references for all attributes.

This line is taken from here: JSON Schema: interactive and non interactive validation

I believe you were refering this when you said:

The document you are using to "prove" it should be "phone" is expired by 13 years already... Do you have a better source?

@hasansezertasan hasansezertasan marked this pull request as ready for review November 11, 2023 11:51
pydantic_extra_types/phone_numbers.py Outdated Show resolved Hide resolved
@Kludex Kludex merged commit 843b753 into pydantic:main Nov 11, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants