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

Use pkg_resources to keep the package zip-safe #4

Merged
merged 1 commit into from
Nov 3, 2017
Merged

Use pkg_resources to keep the package zip-safe #4

merged 1 commit into from
Nov 3, 2017

Conversation

superseed
Copy link
Contributor

@superseed superseed commented Oct 17, 2017

This resolves a bug in the packaging of this package, which I encountered after Faker started depending on it recently.

When text_unidecode is installed manually from a source distribution, in a PYTHONDONTWRITEBYTECODE=1 environment, it ends up in the shape of a zipfile egg in site-packages, as it is not marked zip-unsafe. This means that importing it will result in an error, because this instruction will try to open a file in what it thinks is a directory, when it is actually a zipfile. The package could be marked as zip-unsafe, but there is an even better way to solve the problem:

from pkg_resources import resource_filename
_data_path = resource_filename(__name__, 'data.bin')

pkg_resources (which is part of setuptools) will take care of unzipping and caching the data file, returning a valid filename which can be consumed.

This changeset also removes two redundant lines from Manifest.in:

  • README.rst as it is part of the default README files
  • src/text_unicode/data.bin because it refers to an non-existent file (the package name has a typo in it), but would be redundant anyway as data.bin is specified as part of package_data in setup.py

EDIT: Corrected code quote as I pasted the wrong line.

@frivoire
Copy link

This fix would be very usefull.
Review and merge would be very appreciated :)

@kmike kmike merged commit 08b3859 into kmike:master Nov 3, 2017
@kmike
Copy link
Owner

kmike commented Nov 3, 2017

Thanks @superseed, it makes sense!

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

Successfully merging this pull request may close these issues.

3 participants