Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

remove support for 'python setup.py test' #89

Merged
merged 1 commit into from
Mar 21, 2022
Merged

remove support for 'python setup.py test' #89

merged 1 commit into from
Mar 21, 2022

Conversation

jameslamb
Copy link
Contributor

Follow-up to #81. This PR proposes removing setup.py metadata that is only required to run python setup.py test.

This project's CI doesn't use that command, and setuptools maintainers consider that command deprecated.

Additions

None

Removals

  • removes tests_require from setup.py
  • removes code used to read requirements-test.txt into setup.py

Changes

  • Just removals

Testing

Ran the following to emulate CircleCI locally.

docker run \
   --rm \
    -v $(pwd):/usr/local/src \
    --workdir /usr/local/src \
   -it circleci/python:3.7 \
        /bin/bash

And then installed hamilton and ran its tests.

# install dependencies
sudo apt install graphviz
python -m venv venv || virtualenv venv
. venv/bin/activate
python --version
pip --version
pip install -r requirements-test.txt
pip install -r requirements.txt
pip install "dask[complete]"

# run tests
python -m pytest --cov=hamilton tests/
python -m pytest graph_adapter_tests/h_dask

Screenshots

None

Notes

From https://pypi.org/project/pytest-runner/#description

Deprecation Notice
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 pypa/setuptools#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 tests_requires option.

And from "Why you shouldn't invoke setup.py directly"

The setuptools team no longer wants to be in the business of providing a command line interface and is actively working to become just a library for building packages.

See the table at https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary, which recommends using pytest (as hamilton already does) instead of running python setup.py test.

Thanks for your time and consideration.

Todos

None

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the TODO link to standards
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Python

  • python 3.6
  • python 3.7

Copy link
Collaborator

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning this up. A nice example of code cruft.

@skrawcz skrawcz merged commit 8f01b26 into stitchfix:main Mar 21, 2022
@jameslamb jameslamb deleted the remove-tests-requires branch March 21, 2022 13:41
gitbook-com bot pushed a commit that referenced this pull request May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants