gtsystem / python-remotezip

Python module to access single members of a zip archive without downloading the full content from a remote web server.
MIT License
111 stars 22 forks source link

convert scripts to console_scripts entry_points #21

Closed jhkennedy closed 11 months ago

jhkennedy commented 11 months ago

:wave: I maintain the conda-forge recipe for remotezip and this PR is to support that, and some users downstream (e.g., https://github.com/asfadmin/Discovery-asf_search/issues/240)

Over on the conda side of the house, we can build pure python packages like remotezip as "noarch" packages to get maximum build portability. But, there are a few constraints, that prevent remotezip from being distributed as a noarch package.

Specifically, remotezip using scripts in setup.py prevents a noarch build: https://github.com/gtsystem/python-remotezip/blob/master/setup.py#L19

largely because it's not as portable, doesn't work well on Windows, and is not recommended by the Python Packaging Authority:

Although setup() supports a scripts keyword for pointing to pre-made scripts to install, the recommended approach to achieve cross-platform compatibility is to use console_scripts entry points

console_script entry_points are well supported on the conda-forge side, and since the bin/remotezip script is a python script, it can be easily converted.

This PR converts the remotezip CLI from a python script to a console_script entry_points, making remotezip a single-file "package".

I would not expect this to be noticeable by users as pip will still install a remotezip script with the same CLI interface.

gtsystem commented 11 months ago

Tests are failing due to the new imports. Can you add these dependencies in https://github.com/gtsystem/python-remotezip/blob/master/.github/workflows/python-package.yml#L28 ?

jhkennedy commented 11 months ago

Tests are failing due to the new imports. Can you add these dependencies in https://github.com/gtsystem/python-remotezip/blob/master/.github/workflows/python-package.yml#L28 ?

Oh, whoops. @gtsystem, it'd probably be better just to pip install remotezip from the repository checkout so that the runtime dependencies are picked up and the CLI script is available for testing (though I don't see it being tested).

And since tests_require is a deprecated setuptools keyword, moving the test dependencies to a test group in extras_requires. That would be applying these changes:

diff --git a/setup.py b/setup.py
index efebbe5..c6a9551 100644
--- a/setup.py
+++ b/setup.py
@@ -15,7 +15,9 @@ setup(
     long_description=description,
     long_description_content_type="text/markdown",
     install_requires=["requests", "tabulate"],
-    tests_require=['requests_mock'],
+    extras_require={
+        "test": ["requests_mock"],
+    },
     entry_points={
         'console_scripts': ['remotezip = remotezip:main']
     },
diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml
index c56f2c7..1b1f64d 100644
--- a/.github/workflows/python-package.yml
+++ b/.github/workflows/python-package.yml
@@ -25,12 +25,13 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install requests requests_mock wheel
+        python -m pip install '.[test]'
     - name: Test
       run: |
         python test_remotezip.py -v
     - name: Package
       run: |
+        python -m pip install wheel
         python setup.py bdist_wheel
     - name: Archive artifact
       uses: actions/upload-artifact@v2

I've confirmed the tests will pass with the above changes. I can push those changes if you like?

gtsystem commented 11 months ago

I've confirmed the tests will pass with the above changes. I can push those changes if you like?

Yes thanks

jhkennedy commented 11 months ago

Pushed!

gtsystem commented 11 months ago

Looks like python 2.7 cannot be test any longer. Can you remove it from https://github.com/gtsystem/python-remotezip/pull/21/files#diff-ee49282f461b4c8ad179f79dd5bcdf93124561074c64a771366caf93e99b9320R18 ?

jhkennedy commented 11 months ago

@gtsystem done! I also set fast-fail to false so tests won't be canceled if one fails.

gtsystem commented 11 months ago

Merged, thanks again!

jhkennedy commented 11 months ago

Fantastic! Thanks for your help @gtsystem !

jhkennedy commented 11 months ago

@gtsystem can you push this up to PyPI? Is there anything else you'd like done first that I can help with?

gtsystem commented 10 months ago

@jhkennedy, sorry for the delay, just did. Can you check https://pypi.org/project/remotezip/0.12.2/ ?

jhkennedy commented 10 months ago

@gtsystem Looks good to me! I was able to pip install it, the entry point ran fine on the CLI and I could load in and use it from the python interpreter.

Thank you very much for your help!