Skip to content

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
@dangillet dangillet force-pushed the fix-get-default-locale-path branch from af19aaf to ff870da Compare September 23, 2024 07:34
Fixes python-humanize#105

Use importlib.resources available since Python 3.9 to find the location
of the locale folder containing the translations.
@dangillet dangillet force-pushed the fix-get-default-locale-path branch from ff870da to 9627694 Compare September 26, 2024 16:17
@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

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
@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