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

Cleanup requirements a little and remove setup.cfg file. #64

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

mithro
Copy link
Contributor

@mithro mithro commented Feb 21, 2021

setup_requires needs to be specified in both setup.py and pyproject.toml.

The requirements.txt file should use -e . to install the module (and thus get the requirements). If a local or non-PyPi version of a module should be used to satisfy a requirement for either setup_requires or install_requires then it should also be specified in the requirements.txt file.

Using setup.py is much clearer than setup.cfg and it looks like the file is going to get replaced by pyproject.toml in the future, see pypa/setuptools#1688

Signed-off-by: Tim 'mithro' Ansell [email protected]

@mithro mithro force-pushed the setupcfg branch 13 times, most recently from de4f839 to 3942eca Compare February 21, 2021 22:41
requires = ["setuptools", "wheel", "cython"]
requires = [
"cython",
"setuptools>=42",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Worth adding a comment on why >=42?

entry_points={
'console_scripts': ['fasm=fasm.tool:main'],
},
# Requirements
python_requires=">=3.6",
setup_requires=[ # WARNING: Must be kept in sync with pyproject.toml
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think setup_requires is deprecated really, because of chicken/egg problem.

@mithro mithro mentioned this pull request Feb 24, 2021
@mithro mithro force-pushed the setupcfg branch 14 times, most recently from b1bfb1d to 2137848 Compare March 2, 2021 01:49
@mithro mithro force-pushed the setupcfg branch 6 times, most recently from dfba517 to 0791037 Compare March 3, 2021 03:24
@graingert
Copy link

graingert commented Mar 3, 2021

setup.py is not recommended. While pyproject.toml is preferred to setup.cfg, setup.cfg is still preferred to setup.py:

https://setuptools.readthedocs.io/en/latest/userguide/quickstart.html#transitioning-from-setup-py-to-setup-cfg

@mithro mithro force-pushed the setupcfg branch 7 times, most recently from 634e5c7 to 1c65229 Compare March 8, 2021 07:31
`setup_requires` needs to be specified in *both* `setup.py` and
`pyproject.toml`.

The `requirements.txt` file should use `-e .` to install the module (and
thus get the requirements). If a local or non-PyPi version of a module
should be used to satisfy a requirement for either `setup_requires` or
`install_requires` then it should also be specified in the
`requirements.txt` file.

Using `setup.py` is much clearer than `setup.cfg` and it looks like the
file is going to get replaced by `pyproject.toml` in the future, see
pypa/setuptools#1688

The generated Cython C file is included in the source distribution, so
only people building from the git repository need to have Cython
installed (fixing import issue).

Signed-off-by: Tim 'mithro' Ansell <[email protected]>
Signed-off-by: Tim 'mithro' Ansell <[email protected]>
Signed-off-by: Tim 'mithro' Ansell <[email protected]>
Signed-off-by: Tim 'mithro' Ansell <[email protected]>
Steps are;
 1) Get Python + PIP.
 2) Run `update_version.py`
 3) Install package's system dependencies.
 4) Install package.

Signed-off-by: Tim 'mithro' Ansell <[email protected]>
 * Create a composite action for setting up the system.
 * Expand `check-install.yml` to check that installing works from all
   of;
    - GitHub
    - Checked out locally with `python setup.py install`
    - Checked out locally with `pip install -e .`
    - Building and then installing wheels.
 * Remove `wheel.yml` as covered by `check-install.yml`'s building and
   then installing wheels.

Signed-off-by: Tim 'mithro' Ansell <[email protected]>
 - Use the new mithro/actions-includes for common functionality.
 - Refactor common functionality into their own include actions.

Signed-off-by: Tim 'mithro' Ansell <[email protected]>
@mithro mithro force-pushed the setupcfg branch 4 times, most recently from 6104e2a to 066abb3 Compare March 8, 2021 08:41
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