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

Move to pyproject.toml package file #4019

Closed
wants to merge 30 commits into from
Closed

Move to pyproject.toml package file #4019

wants to merge 30 commits into from

Conversation

benjaminpkane
Copy link
Contributor

Building on top of #3821, this PR moves the main fiftyone package description from a setup.py file to the now recommended pyproject.toml file.

Changes

Hatchling is now used as the build backend for the project. This requires Python 3.8+, so this change will implicitly drop support for Python 3.7 when installing from source

The OpenCV dependency is still dynamically chosen (see dependencies.py) when installing from source. See #1494

Builds themselves have been improved with the hatch.toml file, which is more prescriptive about what files to include. Specifically, the App static files, and documentation recipes and tutorials.

Other packages can be transitioned to pyproject.toml descriptions if this change is accepted!

@benjaminpkane benjaminpkane added setup Package/system setup and configuration cleaning Code cleaning labels Jan 17, 2024
@benjaminpkane benjaminpkane self-assigned this Jan 17, 2024
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (22cd8a0) 15.98% compared to head (cc56ced) 15.98%.
Report is 20 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4019   +/-   ##
========================================
  Coverage    15.98%   15.98%           
========================================
  Files          732      732           
  Lines        82054    82054           
  Branches      1110     1110           
========================================
  Hits         13115    13115           
  Misses       68939    68939           
Flag Coverage Δ
app 15.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@benjaminpkane benjaminpkane marked this pull request as draft January 17, 2024 21:59
@swheaton
Copy link
Contributor

@benjaminpkane appreciate the constant eye toward modernization.
I trust your knowledge of python packaging, mine is limited. Wheel? Egg? setuptools.py?? 😭

  • Are we using hatch as well as hatchling? Or is hatch just kind of a superset that may include hatchling?
  • Were there any alternatives to hatch/hatchling? Why'd you choose it over others if so? In their docs I only see comparison to setuptools and no other alternatives.
  • What potential gotchas do you foresee with this? As we have seen, poetry for example, sounds nice but has some snags we've tripped on over time.

@swheaton
Copy link
Contributor

These may or may not be related useful references.
https://sinoroc.gitlab.io/kb/python/packaging_tools_comparisons.html#build-back-ends
https://packaging.python.org/en/latest/guides/modernize-setup-py-project/#modernize-setup-py-project

As to python3.8 requirement, it does not seem so bad since it's only for building from source which doesn't happen so often. We'll still want to float it around a bit before committing though.

@benjaminpkane
Copy link
Contributor Author

I'd be in favor of officially dropping 3.7 with this change. Still need to fix documentation builds, so leaving it as a draft for now.

@benjaminpkane appreciate the constant eye toward modernization. I trust your knowledge of python packaging, mine is limited. Wheel? Egg? setuptools.py?? 😭

  • Are we using hatch as well as hatchling? Or is hatch just kind of a superset that may include hatchling?
  • Were there any alternatives to hatch/hatchling? Why'd you choose it over others if so? In their docs I only see comparison to setuptools and no other alternatives.
  • What potential gotchas do you foresee with this? As we have seen, poetry for example, sounds nice but has some snags we've tripped on over time.
  1. We are only using hatchling which is a the build backend component of hatch. I experimented with hatch for developer installs, but seems unnecessary
  2. hatchling is what the packaging tutorial currently uses as a default. setuptools is older / more flexible but also harder to maintain. The single hatch.toml simplifies the build description.
  3. The only gotchas beyond what is described above is ensuring the contents of the distribution (now defined by hatch.toml) do not break installations current v0.23.2 dist and this PR's latest dist artifact

@benjaminpkane benjaminpkane marked this pull request as ready for review February 21, 2024 18:25
@benjaminpkane benjaminpkane marked this pull request as draft March 29, 2024 19:41
@benjaminpkane
Copy link
Contributor Author

Closing until the team has more bandwidth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleaning Code cleaning setup Package/system setup and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants