feat: Use hatchling as the build backend #349

matthewfeickert · 2024-03-22T06:09:43Z

Resolves Support Python 3.12 #344
Resolves Is the ServiceX Client a library or an application? #346

If accepted will supercede other open PRs:

Description

fix: Use 'isinstance' for type checks

Avoids flake8 error: E721 do not compare types, for exact checks use is / is not, for instance checks use isinstance(). This is required to pass linting.

feat: Use hatchling as the build backend

The motivation here is that poetry solves are quite difficult to debug with and I would say are quite often wrong. @ponyisi had to do quite a bit of work to tune things in poetry to get PR #348 passing at all, while just providing lower bounds for the hatchling build backend to communicate with the dependency resolvers of pip and uv resulted in trivial solves once it was understood that the pydantic v2.0 API is not supported.

poetry was introduced in d10d159 and I can't find any PRs or GitHub Issues where the decision to adopt it was discussed, so there may be reasons that I don't understand for its adoption. However, if the main motivation was to use its lock file feature I would suggest either just using uv pip compile (super fast!)

# cold cache
$ time uv pip compile --all-extras --generate-hashes --output-file requirements.lock pyproject.toml
...
real	0m0.942s
user	0m0.243s
sys	0m0.319s
# warm cache
$ time uv pip compile --all-extras --generate-hashes --output-file requirements.lock pyproject.toml
...
real	0m0.127s
user	0m0.082s
sys	0m0.106s

or if you want a fully integrated system using pdm.

Switch to use hatchling as the build backend from poetry.
- Provide lower bounds for all dependencies. For dependencies that are used but are dependencies of other dependencies, and so have their versions controlled by them, list without a lower bound.
- Add upper bound on pydantic dependency as the v2.X API change is known to be fully breaking.
- Add missing databinder dependencies of rich and nest-asyncio.
- Remove the jupyter metapackage as a dependency as it obscures the actual dependencies and is not a runtime requirement.
- Support Python 3.8+.
Add 'develop' extra that includes the 'pandas', 'test', and 'docs' extras.
Add PyPI metadata to pyproject.toml.
Remove poetry.lock.

ci: Add Python 3.12 testing to CI

Use pip to install dependencies.
Only install the required 'servicex[pandas,test]' dependencies to test that the install requirements are correct.
Remove install of caio from sdist, as no longer required.

ci: Use PyPA tooling for publishing

As hatchling has been adopted as the build backend, removing poetry, the publishing workflow needs to be updated to use the standard PyPA tooling of build and twine.
- PyPI's Trusted Publisher technology should be used, but that's a seperate PR.
Use sed to update the version metadata in pyproject.toml.
- Search for 'version =' in pyproject.toml and then replace the contents on that line in between double quotes with the version set.
List the contents of the sdist and wheel for visual checks in the logs.
Upload the distributions to GitHub for recovery or debugging if needed.
Use sphinx to build the docs.

* Avoids flake8 error: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

* Switch to use hatchling as the build backend from poetry. - Provide lower bounds for all dependencies. For dependencies that are used but are dependencies of other dependencies, and so have their versions controlled by them, list without a lower bound. - Add upper bound on pydantic dependency as the v2.X API change is known to be fully breaking. - Add missing databinder dependencies of rich and nest-asyncio. - Remove the jupyter metapackage as a dependency as it obscures the actual dependencies and is not a runtime requirement. - Support Python 3.8+. * Add 'develop' extra that includes the 'pandas', 'test', and 'docs' extras. * Add PyPI metadata to pyproject.toml. * Remove poetry.lock.

* Use pip to install dependencies. * Only install the required 'servicex[pandas,test]' dependencies to test that the install requirements are correct. * Remove install of caio from sdist, as no longer required.

codecov · 2024-03-22T06:11:55Z

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 79.89%. Comparing base (b53c8e4) to head (c6082a9).

Files	Patch %	Lines
servicex/servicex_client.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           3.0_develop     #349   +/-   ##
============================================
  Coverage        79.89%   79.89%           
============================================
  Files               41       41           
  Lines             2253     2253           
============================================
  Hits              1800     1800           
  Misses             453      453

Flag	Coverage Δ
unittests	`79.89% <66.66%> (ø)`

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.

matthewfeickert

These are review comments to hopefully guide reviewers. Please ask questions though!

matthewfeickert · 2024-03-22T06:22:39Z

.github/workflows/ci.yaml

-          # Doing this to insure caio library is installed from source since the wheel is broken for macos
-          poetry run pip install --force-reinstall caio --no-binary caio


No longer required.

matthewfeickert · 2024-03-22T06:23:04Z

pyproject.toml

+description = "Python SDK and CLI Client for ServiceX"
+readme = "README.md"
+license = { text = "BSD-3-Clause" }  # SPDX short identifier
+requires-python = ">=3.8"


Support Python 3.8+.

matthewfeickert · 2024-03-22T06:23:37Z

pyproject.toml

+    "qastle>=0.17",
+    "func_adl>=3.2.6",
+    "requests>=2.31",
+    "pydantic>=1.10,<2.0",  # FIXME: Adopt pydantic 2.0 API


A follow up Issue would be supporting the pydantic v2.0 API.

matthewfeickert · 2024-03-22T06:24:48Z

pyproject.toml

+    "rich>=13.0.0",  # databinder
+    "nest-asyncio>=1.5.7",  # databinder # FIXME: nest-asyncio deprecated


These dependencies are required by servicex.databinder but were not specified.

N.B.: https://github.com/erdewit/nest_asyncio is deprecated and archived! This dependency should be moved off of.

matthewfeickert · 2024-03-22T06:25:23Z

pyproject.toml

+Documentation = "https://github.com/ssl-hep/ServiceX_frontend"
+Homepage = "https://github.com/ssl-hep/ServiceX_frontend"
+"Issue Tracker" = "https://github.com/ssl-hep/ServiceX_frontend/issues"
+"Release Notes" = "https://github.com/ssl-hep/ServiceX_frontend/releases"
+"Releases" = "https://github.com/ssl-hep/ServiceX_frontend/releases"
+"Source Code" = "https://github.com/ssl-hep/ServiceX_frontend"


As there are no public docs yet just list the GitHub for everything for the time being.

matthewfeickert · 2024-03-22T06:25:45Z

pyproject.toml

+develop = [
+    "servicex[pandas,test,docs]",
+]


Add a develop extra to install all other extras with a single command.

matthewfeickert · 2024-03-22T06:30:02Z

Tagging @BenGalewsky, @gordonwatts, @ketan96-m, @ponyisi, @kyungeonchoi for review.

As I didn't open up a GitHub Issue or have any formal discussion with the dev team I fully understand that this might not be of interest. Though I think it significantly simplifies things and makes it so that the ServiceX client can be installed alongside other tools much more easily (c.f. Issue #346 and PR #348 for context on that last statement).

* As hatchling has been adopted as the build backend, removing poetry, the publishing workflow needs to be updated to use the standard PyPA tooling of build and twine. - PyPI's Trusted Publisher technology should be used, but that's a seperate PR. * Use sed to update the version metadata in pyproject.toml. - Search for 'version =' in pyproject.toml and then replace the contents on that line in between double quotes with the version set. * List the contents of the sdist and wheel for visual checks in the logs. * Upload the distributions to GitHub for recovery or debugging if needed. * Use sphinx to build the docs.

BenGalewsky · 2024-03-22T17:20:35Z

pyproject.toml

+# pyproject.toml, .gitignore, any README, any LICENSE, AUTHORS
+include = [
+    "/servicex/",
+    "/tests/"


@matthewfeickert - does this bundle the tests into the wheel? That seems unfriendly if so

@BenGalewsky No it doesn't (and agreed that would be terrible)! hatchling builds the sdist from everything in the repo unless you tell it what you want with include or only-include and then it builds the wheel from the sdist, where it will only take the package contents.

Here's a full example of that:

$ python -m build . * Creating venv isolated environment... * Installing packages in isolated environment... (hatchling>=1.13.0) * Getting build dependencies for sdist... * Building sdist... * Building wheel from sdist * Creating venv isolated environment... * Installing packages in isolated environment... (hatchling>=1.13.0) * Getting build dependencies for wheel... * Building wheel... Successfully built servicex-3.0.0a2.tar.gz and servicex-3.0.0a2-py3-none-any.whl $ python -m tarfile --list dist/servicex-*.tar.gz servicex-3.0.0a2/servicex/__init__.py servicex-3.0.0a2/servicex/_version.py servicex-3.0.0a2/servicex/configuration.py servicex-3.0.0a2/servicex/databinder_models.py servicex-3.0.0a2/servicex/dataset_group.py servicex-3.0.0a2/servicex/dataset_identifier.py servicex-3.0.0a2/servicex/expandable_progress.py servicex-3.0.0a2/servicex/minio_adapter.py servicex-3.0.0a2/servicex/models.py servicex-3.0.0a2/servicex/python_dataset.py servicex-3.0.0a2/servicex/query.py servicex-3.0.0a2/servicex/query_cache.py servicex-3.0.0a2/servicex/servicex_adapter.py servicex-3.0.0a2/servicex/servicex_client.py servicex-3.0.0a2/servicex/types.py servicex-3.0.0a2/servicex/app/__init__.py servicex-3.0.0a2/servicex/app/cache.py servicex-3.0.0a2/servicex/app/cli_options.py servicex-3.0.0a2/servicex/app/codegen.py servicex-3.0.0a2/servicex/app/main.py servicex-3.0.0a2/servicex/app/transforms.py servicex-3.0.0a2/servicex/databinder/__init__.py servicex-3.0.0a2/servicex/databinder/databinder.py servicex-3.0.0a2/servicex/databinder/databinder_configuration.py servicex-3.0.0a2/servicex/databinder/databinder_deliver.py servicex-3.0.0a2/servicex/databinder/databinder_outputs.py servicex-3.0.0a2/servicex/databinder/databinder_requests.py servicex-3.0.0a2/servicex/func_adl/__init__.py servicex-3.0.0a2/servicex/func_adl/func_adl_dataset.py servicex-3.0.0a2/servicex/func_adl/func_adl_dataset_group.py servicex-3.0.0a2/servicex/func_adl/util.py servicex-3.0.0a2/tests/__init__.py servicex-3.0.0a2/tests/conftest.py servicex-3.0.0a2/tests/example_config.yaml servicex-3.0.0a2/tests/example_config_no_cache_path.yaml servicex-3.0.0a2/tests/test_config.py servicex-3.0.0a2/tests/test_databinder.py servicex-3.0.0a2/tests/test_dataset.py servicex-3.0.0a2/tests/test_dataset_identifier.py servicex-3.0.0a2/tests/test_datest_group.py servicex-3.0.0a2/tests/test_expandable_progress.py servicex-3.0.0a2/tests/test_minio_adapter.py servicex-3.0.0a2/tests/test_python_dataset.py servicex-3.0.0a2/tests/test_query_cache.py servicex-3.0.0a2/tests/test_servicex_adapter.py servicex-3.0.0a2/tests/test_servicex_dataset.py servicex-3.0.0a2/.gitignore servicex-3.0.0a2/LICENSE servicex-3.0.0a2/README.md servicex-3.0.0a2/pyproject.toml servicex-3.0.0a2/PKG-INFO $ python -m zipfile --list dist/servicex-*.whl File Name Modified Size servicex/__init__.py 2020-02-02 00:00:00 2420 servicex/_version.py 2020-02-02 00:00:00 96 servicex/configuration.py 2020-02-02 00:00:00 5410 servicex/databinder_models.py 2020-02-02 00:00:00 6270 servicex/dataset_group.py 2020-02-02 00:00:00 3835 servicex/dataset_identifier.py 2020-02-02 00:00:00 3403 servicex/expandable_progress.py 2020-02-02 00:00:00 8271 servicex/minio_adapter.py 2020-02-02 00:00:00 4563 servicex/models.py 2020-02-02 00:00:00 5232 servicex/python_dataset.py 2020-02-02 00:00:00 3334 servicex/query.py 2020-02-02 00:00:00 19972 servicex/query_cache.py 2020-02-02 00:00:00 5189 servicex/servicex_adapter.py 2020-02-02 00:00:00 5542 servicex/servicex_client.py 2020-02-02 00:00:00 10921 servicex/types.py 2020-02-02 00:00:00 1685 servicex/app/__init__.py 2020-02-02 00:00:00 1535 servicex/app/cache.py 2020-02-02 00:00:00 3350 servicex/app/cli_options.py 2020-02-02 00:00:00 1776 servicex/app/codegen.py 2020-02-02 00:00:00 2489 servicex/app/main.py 2020-02-02 00:00:00 2295 servicex/app/transforms.py 2020-02-02 00:00:00 5420 servicex/databinder/__init__.py 2020-02-02 00:00:00 1602 servicex/databinder/databinder.py 2020-02-02 00:00:00 2113 servicex/databinder/databinder_configuration.py 2020-02-02 00:00:00 10671 servicex/databinder/databinder_deliver.py 2020-02-02 00:00:00 3487 servicex/databinder/databinder_outputs.py 2020-02-02 00:00:00 3571 servicex/databinder/databinder_requests.py 2020-02-02 00:00:00 5585 servicex/func_adl/__init__.py 2020-02-02 00:00:00 1535 servicex/func_adl/func_adl_dataset.py 2020-02-02 00:00:00 12484 servicex/func_adl/func_adl_dataset_group.py 2020-02-02 00:00:00 2438 servicex/func_adl/util.py 2020-02-02 00:00:00 4013 servicex-3.0.0a2.dist-info/METADATA 2020-02-02 00:00:00 7544 servicex-3.0.0a2.dist-info/WHEEL 2020-02-02 00:00:00 87 servicex-3.0.0a2.dist-info/licenses/LICENSE 2020-02-02 00:00:00 1499 servicex-3.0.0a2.dist-info/RECORD 2020-02-02 00:00:00 2955 $

BenGalewsky

This is great and glad we can boldly build for 3.12

matthewfeickert · 2024-03-22T17:33:40Z

Thanks for the super fast review and merge, @BenGalewsky! If you or the ServiceX team have any questions on related issues in the future please just ping me and I'll try to help out as much as I can. 👍

matthewfeickert · 2024-03-22T17:36:09Z

(Hm, for some reason my "resolves" and "closes" keywords didn't do their job in the PR body, as the referenced Issues and PRs are still open. :? I'll manually close the ones that I opened.)

matthewfeickert added 3 commits March 22, 2024 00:57

fix: Use 'isinstance' for type checks

0c73a71

* Avoids flake8 error: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

ci: Add Python 3.12 testing to CI

6c90cfa

* Use pip to install dependencies. * Only install the required 'servicex[pandas,test]' dependencies to test that the install requirements are correct. * Remove install of caio from sdist, as no longer required.

matthewfeickert commented Mar 22, 2024

View reviewed changes

matthewfeickert changed the title ~~feat: Use hatching as the build backend~~ feat: Use hatchling as the build backend Mar 22, 2024

BenGalewsky reviewed Mar 22, 2024

View reviewed changes

BenGalewsky approved these changes Mar 22, 2024

View reviewed changes

BenGalewsky merged commit 81cc526 into ssl-hep:3.0_develop Mar 22, 2024

matthewfeickert deleted the feat/use-hatchling-as-build-backend branch March 22, 2024 17:33

This was referenced Mar 22, 2024

Is the ServiceX Client a library or an application? #346

Closed

fix: Add CLI entrypoint #350

Merged

Add pytest tests for CLI API #351

Closed

ponyisi mentioned this pull request Mar 22, 2024

Remove poetry lock, don't use implicit upper bounds on versions, fix Py 3.12 compatibility #348

Closed

BenGalewsky mentioned this pull request Mar 22, 2024

Compatibility with python 3.12 #345

Closed

matthewfeickert mentioned this pull request Mar 26, 2024

Support Python 3.12 #344

Closed

matthewfeickert mentioned this pull request Apr 2, 2024

build: Remove nest-asyncio from core dependencies #356

Merged

		# Doing this to insure caio library is installed from source since the wheel is broken for macos
		poetry run pip install --force-reinstall caio --no-binary caio

		"rich>=13.0.0", # databinder
		"nest-asyncio>=1.5.7", # databinder # FIXME: nest-asyncio deprecated

feat: Use hatchling as the build backend #349

feat: Use hatchling as the build backend #349

Uh oh!

Conversation

matthewfeickert commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

codecov bot commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matthewfeickert left a comment

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert commented Mar 22, 2024

Uh oh!

BenGalewsky Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BenGalewsky left a comment

Choose a reason for hiding this comment

Uh oh!

matthewfeickert commented Mar 22, 2024

Uh oh!

matthewfeickert commented Mar 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

matthewfeickert commented Mar 22, 2024 •

edited

Loading

codecov bot commented Mar 22, 2024 •

edited

Loading

matthewfeickert Mar 22, 2024 •

edited

Loading

matthewfeickert Mar 22, 2024 •

edited

Loading