Migrate to pyproject.toml #379

nirosys · 2024-10-19T00:15:29Z

Issue #, if available: #305

Description of changes:
This PR migrates ion-python away from a setup.py/install.py based project to a PEP 517 compliant build system. By updating the build system we are able to adopt better tooling through the formalized nature of PEP 517 projects, and how tooling can integrate with the build tree.

One tool that we can use is a PEP 517 build system called py-build-cmake which allows us use CMake for building our C extension. By using CMake, we're able to place ion-c into the build tree and include it as a subdirectory, letting py-build-cmake handle the python build flags.

A side effect of this, is that the code layout now splits the C module into src/ and the python code into src-python/.

Building the Package

Building the extension is now a simple:

$ python -m build
* Creating isolated environment: venv+pip...
* Installing packages in isolated environment:
  - py-build-cmake~=0.1.8
* Getting build dependencies for sdist...
...
* Building wheel...
...
[100%] Built target _ioncmodule
-- Installing: /var/folders/ld/sktjjrk132z_f7_95g908dxr0000gr/T/tmp4ekl7bjs/staging/amazon/_ioncmodule.so
Successfully built amazon_ion-0.12.0.tar.gz and amazon_ion-0.12.0-cp312-cp312-macosx_13_0_x86_64.whl
(venv) glitch@147dda5e5395 ~/C/ion-python>

This will build both the sdist (source distribution) and a binary wheel for the current platform & python version. This can be used to ensure the module compiles, but does not install the package. By default everything is built in isolation, using a virtual environment and pip, everything (including the C module) is built in a temporary directory.

C Extension Changes

Prior to this PR when installing the package the C extension would be present under amazon.ion.ionc. Due to a limitation in py-build-cmake (at the time of implementation) the C extension is now under top-level package, and has been renamed _ioncmodule. This is reflected in the simpleion module when testing for the extension's existance.

Dependencies

Prior to pyproject.toml, it was common practice to put dependencies into a text file called requirements.txt. With PEP 517 dependency management has been formalized, along with other metadata that previously could be provided differently depending on which build tools you were actually using.

In pyproject.toml we have a section [project] which includes a dependencies field, containing all of the dependencies needed for the package to work. We also have some optional dependencies that we can install for benchmarking, and/or unit testing. Unit testing and benchmarking are not something that most users would need when installing ion-python, so these dependencies are kept separate in the [project.optional-dependencies] section. Each list in this section is a named set of dependencies or extras that can be installed, but are not installed by default.

pip supports these optional dependencies through PEP 517, and they can be installed with pip using a command like:

$ python -m pip install -e '.[test]'

The above command would install the package in editable mode, which only installs the dependencies. Since we reference test, it includes the optional dependencies named test with the required dependencies. This allows us to keep benchmarking dependencies (like ujson, rapidjson, etc) separate so that our users do not need to install them unless they want to do benchmarking. Likewise, if the user isn't contributing or changing ion-python, there's no need for specific versions of venv and tox, required by ion-python.

Configuration

All of the 3rd party tooling configuration (eg. tox, pytest, etc) has been moved into the pyproject.toml.

Additional Changes

In addition to the pyproject.toml migration, I added in a new function for the C extension. Ion C has had a version API since Oct 2023, that can return the version and revision. I've added a new method within the _ioncmodule package that exposes this functionality:

>>> import amazon._ioncmodule
>>> amazon._ioncmodule.ionc_version()
'v1.1.3 (rev: d61c09a)'
>>>

This should make it easier to determine what Ion C fixes/bugs/etc are present in a given ion-python installation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

… and update paths for ionc headers in extension

…oject, update release workflow, and address max-int digits issue for test vectors

…evel

…tension

nirosys · 2025-01-16T00:50:08Z

Windows build issue:

LINK : fatal error LNK1181: cannot open input file 'm.lib' [C:\Users\runneradmin\AppData\Local\Temp\build-via-sdist-rkldgbwp\amazon_ion-0.13.0\.py-build-cmake_cache\pp310-pypy310_pp73-win_amd64\_ioncmodule.vcxproj]

and I missed the performance regression workflow:

ERROR: Could not open requirements file: [Errno 2] No such file or directory: 'requirements.txt'

…e updated to do the same for baseline once merged)

rmarrowstone

Awesome!

rmarrowstone · 2025-01-19T19:11:58Z

.github/workflows/main.yml

-      - run: pip install --upgrade setuptools
-      - run: pip install -r requirements.txt
-      - run: pip install -e .
+      - run: python -m pip install build virtualenv


Why does this action also install virtualenv but the test-windows does not?

Ah good catch, ty. I'll remove that, we're not using virtualenv in the workflows.

rmarrowstone · 2025-01-19T19:12:28Z

.github/workflows/performance-regression.yml

@@ -15,6 +15,7 @@ env:
  specs: '{command:read,format:ion_text} {command:write,format:ion_text} {command:read,format:ion_binary} {command:write,format:ion_binary}'
  test_data_id: 'generated-test-data'
  run_cli: 'python amazon/ionbenchmark/ion_benchmark_cli.py'
+  run_cli_new: 'python src-python/amazon/ionbenchmark/ion_benchmark_cli.py'


Should we just replace run_cli instead of run_cli_new?

The new run_cli_new is temporary, to allow this PR to run the perf regression workflow. Since it pulls down the current HEAD which has the pre-PR layout, I need to run the benchmark CLI from that revision with run_cli.

Once this PR is merged, then I can follow up with a PR to replace run_cli with run_cli_new, so that the perf regression workflow continues working using the src-python layout for everything.

Makes sense.

rmarrowstone · 2025-01-19T19:15:35Z

C_EXTENSION.md

-      compile                                  setup               import ionc module   
+
+The `amazon.ion.simpleion` module then makes use of this extension when it is available to provide more efficient
+Ion parsing. Importing `amazon._ioncmodule` directly can determine if it is available, however simpleion also provides


reading and writing, loading and dumping, not just parsing is all i mean.

rmarrowstone · 2025-01-19T19:18:16Z

pyproject.toml

+test = [
+   "pytest==8.3.3"
+]
+benchmarking = [


rmarrowstone · 2025-01-19T19:18:43Z

src/CMakeLists.txt

+        COMPONENT python_module
+        DESTINATION ${PY_BUILD_CMAKE_MODULE_NAME})
+
+# install(FILES _ioncmodule.pyi


Ah! Yes, I'll get rid of that and follow up. There is a way to export python stubs for extensions to allow IDEs to present function and type information for an extension's methods. I left that as a reminder, but will follow up with it after this PR.

That would be neat.

rmarrowstone · 2025-01-19T19:18:56Z

src/ioncmodule.c

@@ -1531,6 +1531,12 @@ PyObject* ionc_read(PyObject* self, PyObject *args, PyObject *kwds) {
    return exception;
 }

+PyObject *ionc_version() {


…ve commented cmake code

nirosys added 17 commits January 15, 2025 14:26

Migrate to pyproject.toml, and convert the build to use py-build-cmake

7eb62dd

Switch to pkgutil.extend_path (revisit PEP420, this may be unneeded),…

c5c5590

… and update paths for ionc headers in extension

Remove install.py; Update ion-c to latest (fixes sub-directory builds)

f143a42

Update benchmark-cli to use new python code path

bff1587

Add new pyproject.toml and CMakeLists

36eeb75

Adjust IONC_BUILD_TESTS override

f70105d

Update main github actions workflow

41b60fe

Install build module

a358f91

Lower pyparsing version to support 3.8

f69ecf7

Further migration to pyproject.toml; move 3rd party configs into pypr…

fd2ff2f

…oject, update release workflow, and address max-int digits issue for test vectors

Update main workflow to include test dependencies when installing

d33a4a4

Update extension to link against decNumber as a static lib

56cf105

Update extension name, changed to _ioncmodule and is now at the top-l…

480350e

…evel

Fix version, it got lost in the rebase

eb51696

Remove requirements files; add py-build-cmake cache to gitignore

12427ad

Add ionc_version method to return the version of Ion C used in the ex…

60ab03a

…tension

Quick update pass for C Extension documentation

e6e16a2

nirosys force-pushed the rgiliam/build-rework branch from 635da05 to e6e16a2 Compare January 16, 2025 00:44

nirosys added 4 commits January 16, 2025 13:09

Remove libm from linked libraries for windows build

dddd696

Update regression test to use pyproject for PR source (will need to b…

1d5923b

…e updated to do the same for baseline once merged)

Update package cache to only install dependencies, not our package

2434835

Update benchmark cli's path for PR content to account for updated layout

49ca0eb

nirosys marked this pull request as ready for review January 17, 2025 23:36

nirosys requested a review from rmarrowstone January 17, 2025 23:48

Update virtualenv version to match HEAD's dependabot update

070015a

rmarrowstone approved these changes Jan 19, 2025

View reviewed changes

nirosys added 2 commits January 22, 2025 14:56

Remove unneeded virtualenv install; expand wording on C ext use; remo…

0ce40af

…ve commented cmake code

Address actionlint issues

3fdf19b

nirosys merged commit 85ee6ab into amazon-ion:master Jan 22, 2025
27 checks passed

nirosys mentioned this pull request Jan 23, 2025

Follow-up PR to finish updating the performance regression workflow #401

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to pyproject.toml #379

Migrate to pyproject.toml #379

nirosys commented Oct 19, 2024 •

edited

Loading

nirosys commented Jan 16, 2025

rmarrowstone left a comment

rmarrowstone Jan 19, 2025

nirosys Jan 21, 2025

rmarrowstone Jan 19, 2025

nirosys Jan 21, 2025

rmarrowstone Jan 22, 2025

rmarrowstone Jan 19, 2025

rmarrowstone Jan 19, 2025

rmarrowstone Jan 19, 2025

nirosys Jan 21, 2025

rmarrowstone Jan 22, 2025

rmarrowstone Jan 19, 2025

Migrate to pyproject.toml #379

Migrate to pyproject.toml #379

Conversation

nirosys commented Oct 19, 2024 • edited Loading

Building the Package

C Extension Changes

Dependencies

Configuration

Additional Changes

nirosys commented Jan 16, 2025

rmarrowstone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirosys commented Oct 19, 2024 •

edited

Loading