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 missing countries #32

Merged
merged 6 commits into from
Apr 29, 2023
Merged

Conversation

EssaAlshammri
Copy link
Contributor

as disscused in #11

@yezz123

@yezz123 yezz123 added the enhancement New feature or request label Apr 26, 2023
@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

Great! Thank you @EssaAlshammri

Can you run pip-tools to refresh the dependencies to fix the CI/CD

@EssaAlshammri
Copy link
Contributor Author

EssaAlshammri commented Apr 26, 2023

I'm sorry I didn't grasp that!

@yezz123

@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

Yes you can install pip-tools and run make refresh-lockfiles then also check using pre-commit if there is any linting issues

@EssaAlshammri
Copy link
Contributor Author

Oh yeah I did that and it complained about double quotes.
but there are a handfull countries that include single ones.

should I escape them ?

@EssaAlshammri
Copy link
Contributor Author

EssaAlshammri commented Apr 26, 2023

When I escape single quotes it says:
Q003 [*] Change outer quotes to avoid escaping inner quotes

and when I enclose them with double ones it says:
Q000 [*] Double quotes found but single quotes preferred

currently it looks like this

CountryInfo(
    'DZ',
    'DZA',
    '012',
    'Algeria',
    "People's Democratic Republic of Algeria",
),

@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

@EssaAlshammri we can change the configuration regarding ruff in pyproject.toml

@EssaAlshammri
Copy link
Contributor Author

EssaAlshammri commented Apr 26, 2023

by using

avoid-escape = false

or

inline-quotes = 'single' => inline-quotes = 'double'
this will make lint break on other files

?

@EssaAlshammri
Copy link
Contributor Author

or this way

'pydantic_extra_types/types/country.py' = ['Q000']

@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

or this way

'pydantic_extra_types/types/country.py' = ['Q000']

👍🏻

@EssaAlshammri
Copy link
Contributor Author

should I push the changes made by make refresh-lockfiles ?

@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

should I push the changes made by make refresh-lockfiles ?

yes

@EssaAlshammri
Copy link
Contributor Author

error: Unused "type: ignore" comment from mypy

@yezz123
Copy link
Collaborator

yezz123 commented Apr 26, 2023

error: Unused "type: ignore" comment from mypy

I will fix it later, its regarding some changes in the logic 🙏🏻

Thank you @EssaAlshammri

@EssaAlshammri
Copy link
Contributor Author

thank you :)

test aren't passing too

@EssaAlshammri
Copy link
Contributor Author

EssaAlshammri commented Apr 26, 2023

side note

CountryInfo(
            "XK",
            "UNK",
            "",
            "Kosovo",
            "Republic of Kosovo",
        ),

this country doesn't have a numeric code
and it's not listed in https://www.iso.org/iso-3166-country-codes.html

I guess something has to be done either delete it or write something in that field to indecate it doesn't have one.

  • it's the only country that isn't listed in iso.org

@yezz123 yezz123 enabled auto-merge (squash) April 29, 2023 18:43
@yezz123 yezz123 merged commit 4fb9269 into pydantic:main Apr 29, 2023
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.

there seem to be a lots of changes here not related to adding countries, and changing quotes is making this change very hard to review.

Also this doesn't really follow what I suggested in #24.

@yezz123 please hold off on merging unless the change is trivial. I think really we need to revert this, or revert a lot of the changes in main.

Comment on lines +96 to 98
field_schema = {} # type: ignore
field_schema.update(type='string', format='color')
return field_schema
Copy link
Member

Choose a reason for hiding this comment

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

this should just be return {'type': 'string', 'format': 'color'}


from pydantic_core import PydanticCustomError, core_schema

from pydantic.annotated import GetCoreSchemaHandler

T = TypeVar("T")
Copy link
Member

Choose a reason for hiding this comment

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

why are we using double quotes in this file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants