Support Python packaging #1720
Support Python packaging #1720
Conversation
I had exactly the same question. Would be nice if we could put everything in a subdirectory. Where do you see problems? Pointing pip into the subdirectory? |
That (I have never tried yet), and there may be problems also in getting setuptools to include files from a parent directory of |
Nope, worked first try (non-rigorously tested on my setup). The syntax for requiring the package is |
|
Is it not possible to put all the Python package related files into a separate repo and reference GridTools zips from GitHub releases from there? Then we wouldn't have to put anything into the main GridTools repo, although I'm not sure if this is what we want at all. |
Should be possible. The cmake pypi distro is done like that. It only makes sense, though, if we get something back for the added complexity. Do you see any benefits? In terms of added complexity, how would you request a specific version (commit) of the gridtools library if the python package is decoupled like that? |
| session.log("installed gridttols sources") | ||
| version_path = source_path / "version.txt" | ||
| setup_path = pathlib.Path(".") / "setup.cfg" | ||
| config = configparser.ConfigParser() |
There was a problem hiding this comment.
Probably you're already aware, but configparser removes all comments and formatting from the original file. It's not very important in this case, though, but it is not nice. The only alternative I know which preserves format is ConfigUpdate (https://pypi.org/project/ConfigUpdater/) but is a third-party project
| prepare(session) | ||
| dist_path = session.cache_dir.joinpath("dist").absolute() | ||
| workdir = pathlib.Path(".").absolute() | ||
| session.install("build[virtualenv]") |
There was a problem hiding this comment.
Just a comment, maybe it would be good to add all packages used or installed here (build, cmake, ...) to the list of build_system.requires in pyproject.toml to make the dependencies explicit.
There was a problem hiding this comment.
We could add them for documentation purposes, even they are not build requirements anymore. They are pre-build requirements. In the PEP-517 conformant isolated build environment, they don't have to be present.
A compromise might be to add them commented-out, I guess?
.python_package/setup.cfg
Outdated
| name = gridtools-cpp | ||
| author = ETH Zurich | ||
| author_email = gridtools@cscs.ch | ||
| license_files = LICENSE |
There was a problem hiding this comment.
I would like to add a comment here to document the fact that the License file is being copied in the building process... (but configparser would delete the comment later 😓 )
.python_package/setup.cfg
Outdated
| project_urls = | ||
| Source Code = https://github.com/GridTools/gridtools | ||
| platforms = any | ||
| version = 2.2.1a2 |
There was a problem hiding this comment.
Same as above, I would like to add a comment here to document the fact that the version is actually updated from the main gridtools version file... (but configparser would delete the comment later 😓 )
There was a problem hiding this comment.
We can solve these by generating setup.cfg from setup.cfg.in, which would have the comments. Only question is whether setup.cfg should still be versioned (since it has tool config as well).
.python_package/setup.cfg
Outdated
| Development Status :: 5 - Production/Stable | ||
| License :: OSI Approved :: BSD License | ||
| Intended Audience :: Science/Research | ||
| Operating System :: OS Independent | ||
| Programming Language :: Python :: 3 :: Only |
There was a problem hiding this comment.
Please, sort the classifiers alphabetically. Optionally, you might want to add the same topic classifiers used in gt4py:
Topic :: Scientific/Engineering :: Atmospheric Science
Topic :: Scientific/Engineering :: Mathematics
Topic :: Scientific/Engineering :: Physics
.python_package/setup.cfg
Outdated
|
|
||
| [options.package_data] | ||
| * = | ||
| *.hpp |
There was a problem hiding this comment.
Also optional, we could add the license file to the package sources, since the python package is some kind or source distribution
There was a problem hiding this comment.
It is already included in the distribution. Can be found after install in the usual location .../site-packages/gridtools_cpp-XXX.dist-info/LICENSE.
Currently in proof of concept mode. Uses
skbuildto install the gridtools headers and cmake files into the pip package's data folder.Missing in order to be considered for merging:
Feedback would be nice on:
How many more infrastructure files are acceptable?(non-issue, now that all is contained int a subdirectory)Is it worth it to explore if the whole root of the python package can be nested away (might be complicated)?(turns out to be easy)