-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update for PyPI release #18
Conversation
Try an installation with |
pyproject.toml
Outdated
description = "Train and evaluate weather/climate model emulators" | ||
readme = "README.md" | ||
requires-python = ">=3.9" | ||
license = {file = "LICENSE"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to move the pyproject.toml
to the top level in order to dynamically link the README
and LICENSE
files. I did try to leave it under fme
, but it's not permitted to access parent directories for dynamic builds.
@mcgibbon doesn't recall if there was a requirement that setup.py
and other files exist in fme
and not at the top level, but did mention that it would make things more difficult for having multiple packages. Could be something worth discussing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does fundamentally change our repo so that it is now the repo of a single python package, as opposed to being a repo containing a single python package (but which could contain more). Probably worth discussing in our weekly fme sync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would lean to maintaining the current repo structure. Maybe we could duplicate the LICENSE within fme
and make an fme
-specific README within that folder? The benefit of current structure is we can have the scripts folder with data processing etc scripts that are distinct from the fme package.
pyproject.toml
Outdated
|
||
[project] | ||
name = "fme" | ||
version = "2024.10.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This version number was just for testing things out on testpypi. I'm thinking we should use the 2023.12.0
version that's tagged in the repo for actual pypi, but I wanted to check before actually committing a version.
rm -rf dist | ||
python -m build | ||
|
||
deploy_pypi: build_pypi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User needs an API key to push, so it's safe to include in make target (i.e., can't accidentally push without access, and pushing same version fails)
Although installing doesn't fail, and I can import
|
pyproject.toml
Outdated
|
||
[project] | ||
name = "fme" | ||
version = "2024.10.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure fme.core.version.py
is updated accordingly.
fme/requirements.txt
Outdated
@@ -6,10 +6,10 @@ numpy<2 | |||
wandb | |||
tensorly | |||
tensorly-torch | |||
xarray | |||
xarray>=2023.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly out of curiosity.. was there a particular feature that required this version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the original setup.py python>=3.8
the tests were failing without xarray above that version (which also forced the bump to python>=3.9
). I removed since the python requirement automatically solves the xarray version correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that is helpful explanation. I'm +1 on requiring python 3.9 given EOL for 3.8.
@oliverwm1 Okay, I've moved the An example of what the package will appear as can be seen here: https://test.pypi.org/project/fme/2024.9.0/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I tested the install from test-pypi and it worked for me.
Updates for releases to PyPI. Includes move to
pyproject.toml
the current suggested format for package configuration oversetup.py
. Includes some deployment makefile targets for test/production.https://test.pypi.org/project/fme
Resolves #14