Update deprecated GitHub Actions and add Python versions

[martinrieder]

Running 6 different Python versions (3.7 to 3.12) in parallel now.
NOTE: This is in conflict with #309, but can be resolved easily.

GitHub Actions require an update:

• actions/upload-artifact@v3 is scheduled for deprecation on November 30, 2024.
• Similarly, v1/v2 are scheduled for deprecation on June 30, 2024.
• Updating this comes with a breaking change in upload-artifact@v4:
  Uploading to the same named Artifact multiple times.

Due to how Artifacts are created in this new version, it is no longer possible to upload to the same named Artifact multiple times. You must either split the uploads into multiple Artifacts with different names, or only upload once. Otherwise you will encounter an error.

The artifact .zip files therefore have the python version added to their name.

[martinrieder]

Additionally, I suggest adding the following action: Compare build artifacts from two different commits via github actions

[martinrieder]

Note that I have enable PyLint as an additional GH action in my personal repo:
https://github.com/martinrieder/WireViz/blob/master/.github/workflows/pylint.yml

It reports some errors on the current code base that might need fixing...

************* Module src.wireviz.DataClasses
src/wireviz/DataClasses.py:306:23: E1101: Instance of 'float' has no 'split' member (no-member)
src/wireviz/DataClasses.py:306:23: E1101: Instance of 'int' has no 'split' member (no-member)
************* Module src.wireviz.Harness
src/wireviz/Harness.py:371:131: E1[10](https://github.com/martinrieder/WireViz/actions/runs/9930235008/job/27428731076#step:6:11)1: Module 'wireviz.wv_colors' has no 'default_color' member (no-member)
************* Module src.wireviz.wv_cli
src/wireviz/wv_cli.py:153:4: E[11](https://github.com/martinrieder/WireViz/actions/runs/9930235008/job/27428731076#step:6:12)20: No value for argument 'file' in function call (no-value-for-parameter)
src/wireviz/wv_cli.py:153:4: E1120: No value for argument 'format' in function call (no-value-for-parameter)
src/wireviz/wv_cli.py:153:4: E1[12](https://github.com/martinrieder/WireViz/actions/runs/9930235008/job/27428731076#step:6:13)0: No value for argument 'prepend' in function call (no-value-for-parameter)
src/wireviz/wv_cli.py:153:4: E1120: No value for argument 'output_dir' in function call (no-value-for-parameter)
src/wireviz/wv_cli.py:153:4: E1120: No value for argument 'output_name' in function call (no-value-for-parameter)
src/wireviz/wv_cli.py:[15](https://github.com/martinrieder/WireViz/actions/runs/9930235008/job/27428731076#step:6:16)3:4: E1120: No value for argument 'version' in function call (no-value-for-parameter)

If you think that it is useful to add it here as well, then let me know!

[kvid]

I don't have enough experience with pylint to make well-founded comments about its usefulness for our project. In general, I like tools that warn about possible code errors, but if the number of false positives and false negatives are greater than the true findings, then I'm not sure.

It seems pylint has several reported issues about false positives when the code check isinstance(v, type) before accessing v.attr, and that is a frequent code pattern in the WireViz source code. The two errors from DataClasses.py in your report above are such false positives.

• The error from Harness.py seams to be a real misspelling error that needs fixing.
• The errors from wv_cli.py look real, but why does the ordinary CLI use cases work just fine then? There might be some click "magic" involved that I don't understand.

[martinrieder]

See an interesting article on this topic

TL;DR Recommendation:

• Type checker: Mypy
• Error linter: Pyflakes
• Packaging linter: Pyroma
• Code formatter: Black
• Docstring linter/formatter: docformatter
• Complexity analyzer: Radon

Further reading and a cheat sheet

Personally, I am relying on VScode's Pyright and Pylance, though that is partially closed source, so it is only a conditional recommendation. See DetachHead/basedpyright for details and alternatives.

[amotl]

Hi. On linting and code formatting concerns, I dearly recommend to use Ruff these days. I am sure you will never look back.

[kvid]

• We need to apply such a change ASAP to avoid the GitHub Actions error for each push to the PR branch, that I experience now:
  "This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v2."

• I want to squash merge this into master as it is, but not yet create a new release. IMHO, that should be OK, because no files used by running code is changed, but please argue against me or suggest alternatives.

• Adjusting which Python versions to test can be done later in PR Python version update #309. We can keep the 6 tests present here now, to gain some experience.

• Adding any additional GH actions can be moved to a separate PR.

• dev and branches in open PRs can then either be rebased on top of the new master or cherry-pick the squash commit to enable using the updated workflow actions.

Do you agree, @martinrieder, @formatc1702, @amotl, and any others with an opinion?

[formatc1702]

I have very limited experience with GH Actions, linters, etc. but here are my main points:

• We need to apply such a change ASAP to avoid the GitHub Actions error for each push to the PR branch, that I experience now:
  "This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v2."

• I want to squash merge this into master as it is, but not yet create a new release. IMHO, that should be OK, because no files used by running code is changed, but please argue against me or suggest alternatives.

I see no problem here, go ahead 👍

• dev and branches in open PRs can then either be rebased on top of the new master or cherry-pick the squash commit to enable using the updated workflow actions.

Cherry-picking should be enough to avoid rebase hell ;)

TL;DR Recommendation:
* Type checker: Mypy
* Error linter: Pyflakes
* Packaging linter: Pyroma
* Code formatter: Black
* Docstring linter/formatter: docformatter
* Complexity analyzer: Radon

Are you suggesting use of these tools should be enforced for all contributors? Would this increase the difficulty for new contributors? As someone who has not used these tools (beside black) I can't gauge the usefulness/necessity here.

[kvid]

@formatc1702 - Thank's for responding!

I have very limited experience with GH Actions, linters, etc.

The same goes for me, I'm afraid (except isort & black).

but here are my main points:

• We need to apply such a change ASAP to avoid the GitHub Actions error for each push to the PR branch, that I experience now:
  "This request has been automatically failed because it uses a deprecated version of actions/upload-artifact: v2."
• I want to squash merge this into master as it is, but not yet create a new release. IMHO, that should be OK, because no files used by running code is changed, but please argue against me or suggest alternatives.

I see no problem here, go ahead 👍

Thank's - I'll do this within in a couple of days, unless someone else present a better alternative before then.

• dev and branches in open PRs can then either be rebased on top of the new master or cherry-pick the squash commit to enable using the updated workflow actions.

Cherry-picking should be enough to avoid rebase hell ;)

I see your point, but I think we can let the PR authors decide for each case. In some cases, getting also all the latest bugfixes might be worth handling a few conflicts.

TL;DR Recommendation:

• Type checker: Mypy
• Error linter: Pyflakes
• Packaging linter: Pyroma
• Code formatter: Black
• Docstring linter/formatter: docformatter
• Complexity analyzer: Radon

Are you suggesting use of these tools should be enforced for all contributors? Would this increase the difficulty for new contributors? As someone who has not used these tools (beside black) I can't gauge the usefulness/necessity here.

@martinrieder must answer for his suggestions himself, but I suggest moving any additional actions to new PR(s).

[kvid]

This update of GitHub Actions has been merged into master. I also tried cherry-picking this commit to #416, and the new actions were executed successfully, but two old outdated actions (build 3.7 and build 3.8) seem still to be executed also, and since those fail, it blocks merging. How can I disable old actions after updating old branches with these new actions? @martinrieder @amotl @tobiasfalk @formatc1702 or anyone else?

Could my problem be related to this? https://github.com/orgs/community/discussions/26256

[tobiasfalk]

I do not know that much about github, but one solution could be to "archive" them.
https://graphite.dev/guides/archive-git-branch

[kvid]

@tobiasfalk wrote:

I do not know that much about github, but one solution could be to "archive" them. https://graphite.dev/guides/archive-git-branch

Thank you for responding quickly!

I've never used this archive command, but from the documentation, it seems to be a way to create a backup copy with commit history of a part of the local repository. If I understood it correctly, how can that help with the problem I described above?

[tobiasfalk]

As said I do not have much experience with github and guthub actions.
My thought is that these actions are activated in som gold branch and by archiving them they are disabled.

[kvid]

This accepted answer seems relevant: https://stackoverflow.com/questions/67743583/how-can-i-stop-github-actions-from-requiring-old-nodejs-builds

I find the old actions as required before merging into dev or master, and it seems I can remove them as required, but I don't find the new actions to replace them. What am I missing?