-
Notifications
You must be signed in to change notification settings - Fork 207
Add a upstream workflow with a dispatch trigger; follow SPEC0 #1245
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
Add a upstream workflow with a dispatch trigger; follow SPEC0 #1245
Conversation
|
@maxrjones I was think about removing fsspec and NetCDF support to native titiler.xarray and move to obstore + Zarr-python This will mean that titler-xarray would only support python>=3.11 which seems ok |
Co-authored-by: Vincent Sarago <[email protected]>
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.
🚀
| description = "Xarray plugin for TiTiler." | ||
| readme = "README.md" | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.11" |
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.
@maxrjones there is something I'm not sure to understand here
we set python>=3.11 but CI tests still run 3.10 in the matrix without any issue 😬
cc @hrodmn
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 think the tests are actually using a newer python in the uv virtual environment than the runner python specified in the matrix. Looking at https://github.com/developmentseed/titiler/actions/runs/18791545420/job/53622773007, I see:
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.12.3-final-0 ________________
I can take a more detailed look later in the day if helpful.
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.
ah @vincentsarago I think you are missing these lines in your primary CI workflow to tell uv which python version to use:
https://github.com/maxrjones/titiler/blob/4b83b0009699626152ba1e2f8cbb671a369b686a/.github/workflows/upstream.yml#L25-L26
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.
we do have those line
titiler/.github/workflows/ci.yml
Lines 29 to 41 in fff3608
| python-version: ['3.10', '3.11', '3.12', '3.13'] | |
| steps: | |
| - uses: actions/checkout@v5 | |
| - name: Install uv | |
| uses: astral-sh/setup-uv@v7 | |
| with: | |
| version: "0.9.*" | |
| enable-cache: true | |
| - name: Set up Python ${{ matrix.python-version }} | |
| run: uv python install ${{ matrix.python-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.
#1254 shows the difference in behavior between the two configurations. the existing method does not seem to persist the specified python version across steps
Requires #1244 to be merged first.
This PR adds an upstream CI workflow to check that new changes in xarray and Zarr do not break titiler.xarray functionality, which includes a workflow dispatch option.
It also moves the titiler.xarray development dependencies to a dependency group rather than the optional dependencies, such that the group isn't published on PyPI.
In order for this to work, we must also follow SPEC0 which is adopted by Xarray and will soon be adopted by Zarr. IMO this is a good idea since the Zarr and Xarray libraries are still ongoing rapid improvements. The core titiler built on the more stable gdal/cog landscape could still support older python versions.