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

Fix finding location of translations #204

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

dangillet
Copy link
Contributor

@dangillet dangillet commented Sep 22, 2024

Fixes #105

Changes proposed in this pull request:

  • Use importlib.resources available since Python 3.9 to find the location of the locale folder containing the translations.

@hugovk hugovk added the changelog: Fixed For any bug fixes label Sep 22, 2024
Fixes python-humanize#105

Use importlib.resources available since Python 3.9 to find the location
of the locale folder containing the translations.
@dangillet
Copy link
Contributor Author

I tried to use __package__ initially, but the docs suggest to use __spec__.parent instead. https://docs.python.org/3/reference/import.html#package__

The tests cover when __spec__ is None or even if it's missing. As __spec__.parent is read-only , I cannot make a test which sets a different value for it.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Let's do some tuning to speed up with lazy imports, which can be important for CLIs where you want a quick response time.

Using https://github.com/nschloe/tuna we can visualise the import times:

python -m pip install tuna
python -c "import humanize" && python -X importtime -c "import humanize" 2> import.log && tuna import.log

Here's main:

image

Here's this PR:

image

With the suggestions:

image

src/humanize/i18n.py Show resolved Hide resolved
src/humanize/i18n.py Outdated Show resolved Hide resolved
Improve import times

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@dangillet
Copy link
Contributor Author

Hi,

Thank you for the review. I had not considered the CLI aspect of humanize. That's a good point. Now a CLI user will pay the price of importing importlib.resources at some point obviously. But I can see how

if TYPE_CHECKING:
    import os
    import pathlib

is really useful here.

Thank you for showing me tuna. I did not know this tool, and I've used snakeViz in the past, so it's great to see a better alternative. Also this is the first time I see python -X importtime -c "..." in use. I learnt something new today! 👏

@hugovk hugovk merged commit 19b43d3 into python-humanize:main Oct 2, 2024
25 checks passed
@hugovk
Copy link
Member

hugovk commented Oct 2, 2024

I can also imagine some CLI users don't use i18n so might not run touch this. Even for code that might run more often, lazy imports can make --help a bit faster too.

Yeah, tuna is really nice for visualising import times along with -X importtime :)

Thanks for the PR!

@hugovk hugovk changed the title Fix i18n _get_default_locale_path Fix finding location of translations Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: Fixed For any bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some tests fails with pytest-randomly
2 participants