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

Rewrite the logic to resolve package-relative paths so we can remove the "asset" library #247

Merged

Conversation

klamann
Copy link
Contributor

@klamann klamann commented Oct 23, 2020

Currently we use the asset library in exactly one place to figure out the path to a file inside an installed module. The asset library is GPL licensed and brings with it a few more GPL dependencies. As pyhocon is Apache licensed, this causes issues for pyhocon users that cannot include software with strong copyleft licenses like the GPL.

Also, it introduces a lot of transitive dependencies:

pyhocon==0.3.56
  - asset [required: Any, installed: 0.6.13]
    - aadict [required: >=0.2.2, installed: 0.2.3]
      - six [required: >=1.6.0, installed: 1.15.0]
    - globre [required: >=0.1.5, installed: 0.1.5]
    - six [required: >=1.10.0, installed: 1.15.0]
  - pyparsing [required: >=2.0.3, installed: 2.4.7]

Without asset, this would be reduced to

pyhocon==0.3.56
  - pyparsing [required: >=2.0.3, installed: 2.4.7]

In this PR I re-implemented the functionality of the asset.load() function so we can avoid using the asset library in the first place.

The implementation is really basic, because it relies on imp.find_module from the standard library. I would have preferred to use importlib.util.find_spec, but it's not available in Python 2.7. I also added a bunch of tests (the previous test was pretty much non-functional because it mocked the result of asset.load).

Solves #234

provide tests for the new function resolve_package_path() and the
"include package()" statement in the config
srsly, python 2 is no fun. at all.
@coveralls
Copy link

coveralls commented Oct 23, 2020

Coverage Status

Coverage increased (+0.03%) to 95.354% when pulling aaf8ba8 on klamann:feature/include-package-without-asset into c2fba83 on chimpler:master.

@darthbear
Copy link
Member

Awesome work @klamann! Thanks for doing it!

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