inspirehep / inspire-schemas

Inspire JSON schemas and utilities to use them.
GNU General Public License v2.0
8 stars 26 forks source link

global: addition of countries #397

Closed drjova closed 4 years ago

drjova commented 4 years ago
drjova commented 4 years ago

@michamos updated

drjova commented 4 years ago

@harunurhan offical_name is not part of the US.

harunurhan commented 4 years ago

That became a part of the our logic in the backend, after the US was created probably and needs be taken into account no matter if it was on the US or not. @drjova

harunurhan commented 4 years ago

I don't think it's a refinement issue, and you reimplemented it 3 times, and this particular case even requires reimplementation, and closing the PR is the right way here.

All needed is to make sure not to break the logic you're replacing, which is

def country_name_to_code(country_name):
    try:
        country = pycountry.countries.get(name=country_name)
    except KeyError:
        country = pycountry.countries.get(official_name=country_name)
    except KeyError:
        country = pycountry.countries.get(common_name=country_name)

    return country.alpha_2 if country else None

apparently it's only used from name to code conversion but not the other way.. I don't know why is that like that.. perhaps change is not even required. But as I said, just need to make it sure.

michamos commented 4 years ago

Why can't you add simply add the official_name thing here?

drjova commented 4 years ago

@harunurhan @michamos do you mean as a different dict? The only problem I see this way is what we'll export to the UI.

michamos commented 4 years ago

actually I don't think the official_name thing is needed if we use the same name everywhere. All countries have a name, so official_name doesn't seem necessary.

drjova commented 4 years ago

At the moment we have this logic:

def _get_country_name(country):
    return country.get("common_name") or country["name"]
michamos commented 4 years ago

That's ok, as I said, all countries have a name, which is more human-friendly than official_name.

drjova commented 4 years ago

ok so I'll leave it this way

drjova commented 4 years ago

@harunurhan thank you for your suggestions, I have updated the PR.