CI: Refactor sdist workflow and add setup-python action #46538

jonashaag · 2022-03-28T07:57:06Z

Smaller version of #46493 according to suggestion in #46493 (comment)

closes #xxxx (Replace xxxx with the Github issue number)
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

mroeschke · 2022-03-28T17:09:24Z

Sorry my comment wasn't clear - I was hoping that each bullet point would be a separate PR.

jonashaag · 2022-03-29T10:31:51Z

@mroeschke recycled this PR for sdist refactoring. The action code also covers some more cases not relevant to sdist but relevant to future PRs.

jonashaag · 2022-03-29T10:32:09Z

.github/workflows/sdist.yml

-  build:
-    if: ${{ github.event.label.name == 'Build' || contains(github.event.pull_request.labels.*.name, 'Build') || github.event_name == 'push'}}
+  sdist:
+    #if: ${{ github.event.label.name == 'Build' || contains(github.event.pull_request.labels.*.name, 'Build') || github.event_name == 'push'}}


Need to undo this once GHA is green.

mroeschke · 2022-03-29T16:34:44Z

Is the main sdist refactor to add the setup python action or are there independent cleanups that can be separate from the setup action?

jonashaag · 2022-03-29T17:24:38Z

Good point, I've pushed a reduced version.

.github/actions/setup-python/action.yml

mroeschke · 2022-03-30T01:06:17Z

.github/actions/setup-python/action.yml

+
+          # Python deps
+          pip
+          setuptools<60.0.0


Hmm don't love that these deps (pandas + test) are also specified in yaml files as well (aware over multiple yaml files as well). Any idea on how to combine them?

Before the refactoring they were part of an inline pip install ....

We might be able to auto-generate them from a minimal environment.yml file but not sure if that's better than having a little duplication.

Gotcha. Could this be maintained in a separate file and modified here with the date to invalidate the cache?

Happy to do it, but we'd still need to add the current timestamp (or something derived from it, like current month) to the file because otherwise the cache will only be invalidated when we change the file, which we don't do on a regular basis.

NB: For setup-python a stale cache just means slower installs, not older versions.

Yeah makes sense; this block can create a copy of the environment file and add a date. I think it's just easier to manage/know that our dependencies are in files and not as ad hoc like this.

jonashaag · 2022-04-02T10:09:20Z

@mroeschke let me know if I can make this PR smaller to ease reviewing.

jreback · 2022-04-04T22:59:47Z

.github/actions/setup-python/action.yml

+    # TODO: GH#44980 https://github.com/pypa/setuptools/issues/2941
+    - name: Create temporary requirements.txt
+      run: |
+        # Drop cache at least once per month


why would we cache this at all?

I think it saves a couple of minutes but we can also add it later.

jreback · 2022-04-04T23:00:12Z

.github/actions/setup-python/action.yml

+      run: |
+        cat requirements.txt
+        # TODO https://github.com/numpy/numpy/issues/21196
+        bits32=$(python -c 'import sys; print(int(sys.maxsize > 2**32))')


umm what is this?

It is required for 32 bit builds but we can add that back later

datapythonista · 2022-04-05T00:02:56Z

Thanks for all the nice work with the CI @jonashaag. I see you've got several PRs open, and looks like most of them too big to be merged. Before things start to become too messy, I would suggest if you can create an issue for the refactoring of the CI, with the tasks that you've got in mind, or the final status you're aiming to achieve. And then you can open atomic PRs (without mixing things that can be addressed independently). Also, would be nice to keep closing PRs that aren't relevant anymore (so reviewers don't need to navigate and find the ones that are ready mixed with some that don't require attention anymore).

I understand it may be a bit frustrating to work this way. But in a project as big as pandas, we need to make sure that:

Everybody, not only you, can properly understand what you're trying to achieve (in a single PR, and in the whole refactoring)
PRs are easy to review (and it doesn't take a huge amount of time to review, since every feedback iteration would be very time consuming)
When things go wrong in the future because of changes, it's easy to understand what happened, there are issues explaining what was the general idea, and PRs are atomic, so they can be understood better, and reverted if needed, without having to revert unrelated changes.

Thanks!

github-actions · 2022-05-06T00:05:04Z

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

mroeschke · 2022-06-28T17:24:50Z

Is this still relevant @jonashaag or has this been superseded by your other PRs?

jonashaag · 2022-06-28T18:24:15Z

Superseded.

jonashaag force-pushed the refactor-ci-sdist branch from 0e85e98 to 21573df Compare March 29, 2022 10:28

jonashaag changed the title ~~Refactor CI~~ CI: Refactor sdist workflow and add setup-python action Mar 29, 2022

jonashaag commented Mar 29, 2022

View reviewed changes

Use setup-python action

266b06c

jonashaag force-pushed the refactor-ci-sdist branch from 21573df to 266b06c Compare March 29, 2022 17:24

mroeschke reviewed Mar 30, 2022

View reviewed changes

.github/actions/setup-python/action.yml Show resolved Hide resolved

mroeschke reviewed Mar 30, 2022

View reviewed changes

jonashaag mentioned this pull request Apr 2, 2022

Add setup-conda action #46611

Merged

4 tasks

jonashaag added 2 commits April 4, 2022 13:34

Move pip reqs to file

83481aa

fix

c522db3

jreback added Build Library building on various platforms CI Continuous Integration labels Apr 4, 2022

jreback requested changes Apr 4, 2022

View reviewed changes

jonashaag mentioned this pull request Apr 5, 2022

Meta issue: CI refactoring #46643

Closed

6 tasks

review feedback

180e2b8

github-actions bot added the Stale label May 6, 2022

Merge branch 'main' into refactor-ci-sdist

8ef99ce

jonashaag closed this Jun 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Refactor sdist workflow and add setup-python action #46538

CI: Refactor sdist workflow and add setup-python action #46538

jonashaag commented Mar 28, 2022

mroeschke commented Mar 28, 2022

jonashaag commented Mar 29, 2022

jonashaag Mar 29, 2022

mroeschke commented Mar 29, 2022

jonashaag commented Mar 29, 2022

mroeschke Mar 30, 2022

jonashaag Mar 30, 2022

mroeschke Apr 3, 2022

jonashaag Apr 3, 2022

jonashaag Apr 3, 2022

mroeschke Apr 4, 2022

jonashaag commented Apr 2, 2022

jreback Apr 4, 2022

jonashaag Apr 5, 2022

jreback Apr 4, 2022

jonashaag Apr 5, 2022

datapythonista commented Apr 5, 2022

github-actions bot commented May 6, 2022

mroeschke commented Jun 28, 2022

jonashaag commented Jun 28, 2022

CI: Refactor sdist workflow and add setup-python action #46538

CI: Refactor sdist workflow and add setup-python action #46538

Conversation

jonashaag commented Mar 28, 2022

mroeschke commented Mar 28, 2022

jonashaag commented Mar 29, 2022

Choose a reason for hiding this comment

mroeschke commented Mar 29, 2022

jonashaag commented Mar 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonashaag commented Apr 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Apr 5, 2022

github-actions bot commented May 6, 2022

mroeschke commented Jun 28, 2022

jonashaag commented Jun 28, 2022