Skip to content

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Jun 5, 2025

I also added a dependabot config to keep the GitHub Actions updated.

Note that we may need to tweak the pyproject.toml before merging this.

Closes #12

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 5, 2025

I created the namespace with an empty release. PyPI no longer accepts registering namespace without a package. It is in the IOOS org at https://pypi.org/project/xarray-subset-grid/

@ocefpaf ocefpaf marked this pull request as ready for review June 5, 2025 19:03
@ChrisBarker-NOAA
Copy link
Contributor

Hmm -- presumably we could use pixi to set up the build environment -- but maybe that wouldn't be helpful?

And while you are looking at the CI -- can you figure out how to use variables to set the pixi version, etc in: test.yaml? (or maybe the dependabot can keep those updated?)

@ChrisBarker-NOAA
Copy link
Contributor

Note that we may need to tweak the pyproject.toml before merging this.

What needs tweaking?

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 6, 2025

What needs tweaking?

As I mentioned above, it may need tweaking. We would only know after the GHA runs. Now we know. Both the sdist and the wheel are build and tested in the CI.

Hmm -- presumably we could use pixi to set up the build environment -- but maybe that wouldn't be helpful?

Yeah, it is kind of an overkill. We only need build and twine.

And while you are looking at the CI -- can you figure out how to use variables to set the pixi version, etc in: test.yaml? (or maybe the dependabot can keep those updated?)

You mean the versions of the GitHub Actions? Sure, for security reasons it is recommended to use a fixed released version hash, like in the file I added, and then add the version as a comment. Dependabot will update the Action and the version in automated PRs.

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 6, 2025

@ChrisBarker-NOAA I know it is super convenient to commit directly to main, but that generates a lot of frustration and unexpected merge conflicts for those submitting PRs. If you don't mind, can we adopt the common development workflow of "fork -> branch -> PR"? It would help a lot to get more contributors to the project.

pixi run --environment ${{ matrix.environment }} test
- name: Lint
if: contains(${{ matrix.os }}, "ubuntu") && ${{ matrix.environment }} == "test310"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not working yet. I need to check GHA docs. However, if we go the route proposed in #66, we should remove linting from here anyway.

Copy link
Contributor

@ChrisBarker-NOAA ChrisBarker-NOAA Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keep it the way it was? it worked, and I prefer the lint to be a separate job anyway.

Putting it in the matrix, and then selecting out only part of the matrix seems like complication for no reason.

But if it's made a pre-commit hook, then as you say, a moot point.

@ChrisBarker-NOAA
Copy link
Contributor

can we adopt the common development workflow of "fork -> branch -> PR"

well, that is absolutely the way to go once a project is established and has a release but when there's a lot of churn to get the first one out, it gets in the way -- while it's a challenge to keep your PR up with main, it's even more a challenge if I"d been making changes in yet another branch -- how would you sync easily with that?

Anyway -- we are almost there now -- is this ready to merge, or do you want to do #66 first?

Hmm -- I see that:
tests/test_grids/test_sgrid.py

Is taking a VERY long time on the CI (for two jobs anyway, not sure why it hasn't timed out yet)

That's a bad test anyway -- downloads a big dataset to test on -- I really had a hard time getting the devs to "get" how unit testing is supposed to work.

Until I can fix that with a better test, maybe we should just skip it -- or not run any of the "--online" tests at all.

@ChrisBarker-NOAA
Copy link
Contributor

OK -- I updated the test task to not inlcude the online tests. there's now a "test_all" task that does.

For now, we'll only run the test task in the CI -- the online tests are really more integration tests anyway.

It's in the remove_online_tests branch -- you'll need to merge that with your branch to get this going ...

Or I can merge that into main now, but it seems you didn't want that ...

@ocefpaf ocefpaf mentioned this pull request Jun 6, 2025
@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 6, 2025

Anyway -- we are almost there now -- is this ready to merge, or do you want to do #66 first?

Nope. This one first. I'll rebase and remove the draft status from #66 when it is ready.

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 9, 2025

Or I can merge that into main now, but it seems you didn't want that ...

That is fine if it is via PRs. PRs are more visible, review, etc. I prefer to avoid direct commits to main.

@ChrisBarker-NOAA
Copy link
Contributor

OK -- merged the test fixes in (#69) -- that should be better now.

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 9, 2025

OK -- merged the test fixes in (#69) -- that should be better now.

@ChrisBarker-NOAA this one should be ready. Once merged I'll rebase and finish #68.

@ChrisBarker-NOAA
Copy link
Contributor

Sounds good -- thanks!

@ChrisBarker-NOAA ChrisBarker-NOAA merged commit 1d0de65 into ioos:main Jun 10, 2025
13 of 14 checks passed
@ChrisBarker-NOAA
Copy link
Contributor

So how do we trigger a build / publish?

Make a gitHub release?

@ocefpaf ocefpaf deleted the GHA_pypi branch June 10, 2025 10:55
@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 10, 2025

Make a gitHub release?

Yep. Do you want to mint one?

@ChrisBarker-NOAA
Copy link
Contributor

@ocefpaf : how do we actually push to PyPi?

Does making a github release by hand trigger the build?

@ocefpaf
Copy link
Member Author

ocefpaf commented Jun 11, 2025

@ocefpaf : how do we actually push to PyPi?

Does making a github release by hand trigger the build?

Yep. All you need is to make a GitHub release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pypi release and pipeline
2 participants