tommyod / KDEpy

Kernel Density Estimation in Python
https://kdepy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
584 stars 90 forks source link

Change build action to not automatically publish to PyPI? #153

Closed whitews closed 1 year ago

whitews commented 1 year ago

Tommy,

I noticed the merge into master triggered a published release on PyPI. Looks like any commit to master will do this, is this the intended behavior? Maybe there's a way to trigger this on a tagged release?

tommyod commented 1 year ago

It's intended because I am lazy. If it goes to master, then I am happy enough with it.

If you have a differing opinion, let me know! Willing to discuss this!

If I remember correctly I used to manually push to PyPI. But these days I think anything that can be automated is a plus.

whitews commented 1 year ago

Gotcha! To keep it simple then, how about we create a develop branch and I could fork and submit PRs to that branch, then when you are happy to make a release merge develop to master?

tommyod commented 1 year ago

Sure thing! I created develop now :)

whitews commented 1 year ago

Tommy,

Thanks for the colab invite! FYI, I will stick to only pushing to the develop branch and never mess with the main functionality of the package.

Alright, so it looks like the new CI setup is working as intended so far. I fixed the job names to show both the OS & Python version. I also added a conditional for the wheel building to only publish when triggered from the master branch. This lets us test the wheel builds and store the artifacts to inspect them if needed before publishing an official new version.

One remaining issue is that I don't see the all the new workflows...I think a merge to master is needed for them to show up. This should now be safe and not trigger a PyPI release.

tommyod commented 1 year ago

Sounds good. I hope that the colab invite can help you.

I still want to look over PRs, especially if they introduce major changes. But if it's a minor change and I am slow to respond, you are welcome to merge a PR without waiting for my review. Don't let me get in your way :)

I think the maintenance work and getting good CI/CD is important, but I don't have time to look into it much myself at the moment. I hope to learn more as I watch your great work on this.

whitews commented 1 year ago

Sounds good to me. The CI stuff is important & powerful, but can sometimes be frustratingly cryptic to figure out.

So I'm not seeing the new workflows in the list on the Actions page, only see the "Build & test (develop)" and the "Python CI" ones. Do they show up for you? Not sure if it's a permissions thing for me or if the workflow files have to hit master before they show up.

tommyod commented 1 year ago

Do they show up for you?

Nope. Same for me:

image

whitews commented 1 year ago

Ok, I'll merge develop into master and see if it registers the new workflows.

whitews commented 1 year ago

Hey Tommy,

The new workflows appear to all be working correctly. A couple notes on building the wheels.

I skipped building some Linux wheels (32-bit, musl, PyPy). Looks like many packages have dropped 32-bit Linux wheels and didn't seem necessary to start building them now. The musl & PyPy Linux folks I feel should mostly be fine with building from source.

I thought about doing the same for Windows 32-bit but I think there are still some percentage of these users out there due to Python itself distributing 32-bit installers (even for 64-bit systems) for a while. On the Mac side, the Apple Silicon wheels are now available. And all the platforms have Python 3.12 wheels.

The "Build wheels" workflow allows building the wheels on non-master branches without pushing to PyPI, but will publish if run from the master branch. Though the workflow does have to be manually triggered.

I think that's everything. Do you want the next version to be 1.1.8 or 1.2.0? It's arbitrary I suppose, there aren't any new features but it does have 3.12 support. In either case, if you are happy with how everything looks, all that is left is to merge develop into master, let the tests run, and trigger the build wheels.

tommyod commented 1 year ago

Sounds good. Great work!

Do you want the next version to be 1.1.8 or 1.2.0?

1.1.8 is probably fine.

In either case, if you are happy with how everything looks, all that is left is to merge develop into master, let the tests run, and trigger the build wheels.

Great, please go ahead. :+1:

whitews commented 1 year ago

Version 1.1.8 is published & I tagged a release. Unless an issue pops up, that'll be it for a while. Take it easy Tommy!

tommyod commented 1 year ago

image

Very nice. Thanks for good work!