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

256 all configs within pyprojecttoml for a streamlined build experience #271

Conversation

davidhopkinson26
Copy link
Contributor

@davidhopkinson26 davidhopkinson26 commented Jul 16, 2024

Draft of moving all project configs to pyproject.toml

moved into pyrpoject.toml:

  • build spec (setup.py)
  • ruff.toml
  • . coveragerc

deleted:

  • top level conftest.py
  • bandit.yml

Haven't yet got rid of requirements.txt or requirements-dev.txt.

  • requirements-dev.txt because still using pip in the devcontainer set up and I'm not sure how to get pip to install deps from pyproject.toml dependency spec. Edit: now understand this could simply be done with pip install .[dev] in this case. Keeping pinned requirments-dev.txt as a fixed developer environment is still handy to have.

  • requirements.txt because it seems like build still wants it to exist separately from pyproject.toml which I find strange and frankly I'm quite sure I am missing something.

Might just move over to poetry for env managment and build to resolve the above.

Moved to using pip-tools workflow, defining requirments.txt and requirements.dev by running pip compile on requirements specified in pyproject.toml which resolves the above nicely.

Doing so and fixing the versions of all dependencies has revealed an incompatibility between specified packages and python 3.12: scipy==1.10.1 depends on Python>=3.8,<3.12. Previously scipy was not specified in requirements so presumably a later version of the package was picked up when installing in python 3.12.

Now changed to install dependiencies from pyproject.toml in build pipeline so it creates a compatible build for each python version and will flag on scheduled run if this is not possible.

It is possible to specify different package sepec depending on python version in pyproject.toml example but pip-tools pip-compile will not run for multiple versions of python or even a different version to the one it is run in.

@davidhopkinson26 davidhopkinson26 marked this pull request as ready for review July 17, 2024 11:24
version = "1.3.1"
dependencies = [
"pandas>=1.5.0",
"scikit-learn>=1.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah - answered my own question by seeing that you install directly from the pyproject file.

@adamsardar
Copy link
Contributor

adamsardar commented Jul 19, 2024

OK, I think that this PR is close.

  • setup.py is still in the repo, and it can be fully replaced by settings in pyproject.toml
    • playing on my local linux machine, all I needed to add to pyproject.toml was the section (build would think that the 'profiling' directory was a package otherwise):
    [tool.setuptools]
    packages = ["tubular"]
    
    [tool.setuptools_scm]
    # can be empty if no extra settings are needed, presence enables setuptools_scm
  • version number is still statically set in the config, even though setuptools-scm is installed as part of the build system. See their docs for details, but the TL;DR is to add dynamic = ["version"] under project and then replace tubular/version.pywith a variable set ininit.py`:
try:
    __version__ = version("tubular")
except PackageNotFoundError:
    # package is not installed
    pass

(you need to remove the import of __version__ as you set it dynamically).

  • Delete requirements.txt - it's not needed anywhere now. requirements-dev.txt is used by the devcontainer setup.

With these parameters in place, we can draw the tag version from git and use that to define package version:

$ git tag v2.1.0 #Not a tag in our repo - for illustration
$ python -m build # From within the tubular root dir
* Creating isolated environment: venv+pip...
* Installing packages in isolated environment:
  - setuptools >= 61.0
  - setuptools-scm[toml]>=6.0
  - wheel
* Getting build dependencies for sdist...
[ ... ]
Successfully built tubular-2.1.1.dev0+g3942b60.d20240719.tar.gz and tubular-2.1.1.dev0+g3942b60.d20240719-py3-none-any.whl
$ ls dist/*
dist/tubular-2.1.1.dev0+g3942b60.d20240719-py3-none-any.whl  dist/tubular-2.1.1.dev0+g3942b60.d20240719.tar.gz

(the extra stuff dev0+g3942b60.d20240719 is because I had unstaged code in my repo - as per the versioning schema).

(I tried the above build without requirements.txt/requirements-dev.txt and it all worked fine - I think that is was setup.py pulling them in before).

Otherwise, it looks good.

requirements.txt Outdated Show resolved Hide resolved
@davidhopkinson26
Copy link
Contributor Author

OK, I think that this PR is close.

  • setup.py is still in the repo, and it can be fully replaced by settings in pyproject.toml

    • playing on my local linux machine, all I needed to add to pyproject.toml was the section (build would think that the 'profiling' directory was a package otherwise):
    [tool.setuptools]
    packages = ["tubular"]
    
    [tool.setuptools_scm]
    # can be empty if no extra settings are needed, presence enables setuptools_scm
  • version number is still statically set in the config, even though setuptools-scm is installed as part of the build system. See their docs for details, but the TL;DR is to add dynamic = ["version"] under project and then replace tubular/version.pywith a variable set ininit.py`:

try:
    __version__ = version("tubular")
except PackageNotFoundError:
    # package is not installed
    pass

(you need to remove the import of __version__ as you set it dynamically).

  • Delete requirements.txt - it's not needed anywhere now. requirements-dev.txt is used by the devcontainer setup.

With these parameters in place, we can draw the tag version from git and use that to define package version:

$ git tag v2.1.0 #Not a tag in our repo - for illustration
$ python -m build # From within the tubular root dir
* Creating isolated environment: venv+pip...
* Installing packages in isolated environment:
  - setuptools >= 61.0
  - setuptools-scm[toml]>=6.0
  - wheel
* Getting build dependencies for sdist...
[ ... ]
Successfully built tubular-2.1.1.dev0+g3942b60.d20240719.tar.gz and tubular-2.1.1.dev0+g3942b60.d20240719-py3-none-any.whl
$ ls dist/*
dist/tubular-2.1.1.dev0+g3942b60.d20240719-py3-none-any.whl  dist/tubular-2.1.1.dev0+g3942b60.d20240719.tar.gz

(the extra stuff dev0+g3942b60.d20240719 is because I had unstaged code in my repo - as per the versioning schema).

(I tried the above build without requirements.txt/requirements-dev.txt and it all worked fine - I think that is was setup.py pulling them in before).

Otherwise, it looks good.

Thanks Adam. Must have rushed this a bit because I thought I had removed setup.py already! Done here: 1e0bbf1 also intended to remove setuptools-scm form this PR and add in a follow up but I have got it working in this commit: 1e0bbf1

As a knock on of this I have removed logic in base.py that set a self.version attribute. Can do this with importlib but this attribute doesnt get used anywhere as far as i can tell so I removed to declutter a bit.

adamsardar
adamsardar previously approved these changes Aug 5, 2024
Copy link
Contributor

@adamsardar adamsardar left a comment

Choose a reason for hiding this comment

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

Looks good. A good simplification of the package and good foundations of the next change.

I tried creating a package via the tag mechanism and it worked. We do have to rather hope that the CD process holds, but such is the way with these things often.

Approved.

adamsardar
adamsardar previously approved these changes Aug 5, 2024
Copy link
Contributor

@adamsardar adamsardar left a comment

Choose a reason for hiding this comment

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

Reapproved after Merge Conflict fixes

Copy link
Contributor

@adamsardar adamsardar left a comment

Choose a reason for hiding this comment

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

third time is a charm - _version crept back in there.

@davidhopkinson26 davidhopkinson26 merged commit e3a1376 into main Aug 5, 2024
12 checks passed
@davidhopkinson26 davidhopkinson26 deleted the 256-all-configs-within-pyprojecttoml-for-a-streamlined-build-experience branch August 5, 2024 16:15
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.

All configs within pyproject.toml for a streamlined build experience
2 participants