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

Explicit error if _DEFAULT_LOCALE_PATH is None #132

Merged
merged 3 commits into from
Jun 27, 2020
Merged

Explicit error if _DEFAULT_LOCALE_PATH is None #132

merged 3 commits into from
Jun 27, 2020

Conversation

eldipa
Copy link
Contributor

@eldipa eldipa commented Jun 14, 2020

Try to use __file__ if possible but do not crash if not.

Instead, when activate is used and no explicit path is given, raise an
exception explaining why.

A better alternative could be use resource_path
(https://docs.python.org/3/library/importlib.html#importlib.abc.ResourceReader)
to find the directory of 'locale'.

Unfortunately Python (at least 3.7) only allows to access to files
and not folders so we cannot access to 'locale' (a folder)

Closes #81

Changes proposed in this pull request:

  • Do not crash if __file__ does not exist
  • Raise an exception if the default path is needed

@codecov-commenter
Copy link

codecov-commenter commented Jun 14, 2020

Codecov Report

Merging #132 into master will decrease coverage by 0.71%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master     #132      +/-   ##
===========================================
- Coverage   100.00%   99.28%   -0.72%     
===========================================
  Files            9        9              
  Lines          416      421       +5     
===========================================
+ Hits           416      418       +2     
- Misses           0        3       +3     
Flag Coverage Δ
#GHA_Ubuntu 99.28% <50.00%> (-0.72%) ⬇️
#GHA_Windows 99.28% <50.00%> (-0.72%) ⬇️
#GHA_macOS 99.28% <50.00%> (-0.72%) ⬇️
Impacted Files Coverage Δ
src/humanize/i18n.py 92.50% <50.00%> (-7.50%) ⬇️
tests/test_number.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9aa0d4f...1e4b885. Read the comment docs.

@eldipa eldipa changed the title Use resource_path, leave __file__ as fallback Explicit error if _DEFAULT_LOCALE_PATH is None Jun 15, 2020
Try to use __file__ if possible but do not crash if not.

Instead, when `activate` is used and no explicit path is given, raise an
exception explaining why.

A better alternative could be use `resource_path`
(https://docs.python.org/3/library/importlib.html#importlib.abc.ResourceReader)
to find the directory of 'locale'.

Unfortunately Python (at least 3.7) only allows to access to files
and not folders so we cannot access to 'locale' (a folder)

Closes #81
@eldipa
Copy link
Contributor Author

eldipa commented Jun 15, 2020

The failing test is about formatting. I don't have problems to fix it, but I cannot make the lint work. Is any document about how to setup an environment for development for humanize?

@hugovk
Copy link
Collaborator

hugovk commented Jun 15, 2020

Thanks for the PR!

There's no docs about that, it'd be good to put it into a CONTRIBUTING.md.

Anyway, to do all the linting:

pip install pre-commit
pre-commit install  # optional, this is to enable the pre-commit linting and run when you commit, on just the staged changes
pre-commit run --all-files  # to run on all files now

@hugovk
Copy link
Collaborator

hugovk commented Jun 26, 2020

@eldipa I've created https://github.com/eldipa/humanize/issues/1 to fix the lint. Merging that will update this PR, then we can get a release out soon. Thanks!

Fix Black, rewrap and capitalise
@hugovk hugovk added bug Something isn't working changelog: Fixed For any bug fixes labels Jun 27, 2020
@hugovk hugovk merged commit 976ad4a into jmoiron:master Jun 27, 2020
@hugovk
Copy link
Collaborator

hugovk commented Jun 27, 2020

Thanks!

@hugovk hugovk mentioned this pull request Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog: Fixed For any bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop requiring __file__
3 participants