search

automl / amltk

A build-it-yourself AutoML Framework
https://automl.github.io/amltk/
BSD 3-Clause "New" or "Revised" License
62 stars 4 forks source link

ci: Remove unnecessary Python setup step #285

Closed PGijsbers closed 4 months ago

PGijsbers commented 4 months ago

Reference Issues/PRs

I could've opened a separate issue, but I figured a PR makes it easier to view my suggestion.

What does this implement/fix? Explain your changes.

When looking at this workflow I noticed that there seems to be an extraneous Python setup step in the citizen bump job. As far as I can tell, it serves no purpose. In my work-in-progress workflow I left it out without issue.

Minimal Example / How should this PR be tested?

Run the workflow (on a fork where it is not a trusted PyPI publisher...). I tried it here, but unfortunately the docs step already seems to be failing, so it won't show a successful bump. So that step would need to be resolved first, but it's unrelated to this PR. After that, you should see that indeed the Python setup step is not necessary to execute the bump job and you can release 23 seconds faster. Nifty.

Any other comments?

Made the change from the browser, so it did bypass pre-commit. I assume there is a pre-commit CI running so that if there's a problem it'll be detected before a merge anyway.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

eddiebergman commented 4 months ago

Oh very nice, thanks for this! Always nice to shave off time when watching the release workflow in anticipation ;)

I looked at the docs build step and I am clueless as to why that is failing? I've ran it again on master so hopefully it fails there (hopefully meaning it's reproducbile), otherwise this is a mystery... https://github.com/automl/amltk/actions/runs/9214528781

I will merge it anywho tomorrow morning, I just want to see if there's some actionable signal first.

Side note, what are you using it for? If you need any help regarding mkdocs/mike, or the requirements for this let me know! Something which I'm not sure if documented as that it relies on commitizen and explicitly no one manually updating versions.

eddiebergman commented 4 months ago

Merging, lovely PR <3

I found my culprit to be markdown-exec. I've had to monkey patch this a few times for controlling example rendering in docs with env variables as well as allow for multiprocessing... If you come across any other solutions to runnable markdown for mkdocs, I would be grateful for knowing!

PGijsbers commented 4 months ago

Merging, lovely PR <3

Thanks!

Side note, what are you using it for?

Right now, I want to use it in aiondemand: a Python wrapper for the AI-on-Demand REST API. But realistically I want all "my" Python projects to have an easy build/release workflow eventually.

If you need any help regarding mkdocs/mike, or the requirements for this let me know!

Thanks! So far not really encountering any issues, the projects' documentation and your working reference workflow are good resources :)

If you come across any other solutions to runnable markdown for mkdocs, I would be grateful for knowing!

Not aware of any.