kynan / nbstripout

strip output from Jupyter and IPython notebooks
Other
1.19k stars 94 forks source link

It is recommended to remove pytest-runnner from setup_requires in setup.py #168

Closed pzdkn closed 1 year ago

pzdkn commented 1 year ago

Due to deprecated features, it is recommended to remove pytest-runner from setup_requires in setup.py (source).

pytest-runner depends on deprecated features of setuptools and relies on features that break security mechanisms in pip. For example 'setup_requires' and 'tests_require' bypass pip --require-hashes. See also https://github.com/pypa/setuptools/issues/1684.

It is recommended that you:

Remove 'pytest-runner' from your setup_requires, preferably removing the setup_requires option. Remove 'pytest' and any other testing requirements from tests_require, preferably removing the

This also causes a long trace of error when running nbstripout as a pre-commit hook. Relevant error line after running pre-commit run -a with the recommended yaml entry:

subprocess.CalledProcessError: Command '['/Users/<user_folder>/.cache/pre-commit/repoid22b5j6/py_env-python3.9/bin/python', '-m', 'pip', '--disable-pip-version-check', 'wheel', '--no-deps', '-w', '/var/folders/ln/kgmmx8kn4g50tg08r0hx91800000gn/T/tmpnm00h86e', '--quiet', 'pytest-runner']' returned non-zero exit status 2.

Removing pytest-runner from setup.pysolves the issue.

kynan commented 1 year ago

Thanks for reporting this! What's the minimum test case for me to reproduce this?

pzdkn commented 1 year ago

You could try adding

  - repo: https://github.com/kynan/nbstripout
    rev: 0.6.0
    hooks:
      - id: nbstripout
        additional_dependencies: ["--index-url=https://pypi.org/simple/"]

to .pre-commit-config.yaml and run pre-commit run -a. However, it possibly could also be tied to our infrastructure setup. I think the deprecation warning is already a good reason to change.

kynan commented 1 year ago

Unfortunately it's not that easy: pytest-runner is needed to make python setup.py test correctly invoke pytest, which is currently used for CI via GitHub Actions. I first need to figure out how to refactor the CI.

kynan commented 1 year ago

Turns out this wasn't too hard after all :) Fixed in master, will be released in 0.6.1