Further CI improvements

[darcymason]

Continuing on from #1116, collecting list of further improvements noted there:

• Add new "merge" or scheduled workflow:

  • Add @SimonBiggs pymedphys checks
  • Add macos to matrix
  • Consider adding PyPy for some combinations - maybe a separate issue/PR
  • get GDCM working on Windows versions? - conda only available in Py3.6
  • run full matrix as much as possible, e.g. all deps checks for all Python versions

• Convert sphinx doc build from Circle to github actions workflow

• Linting improvements:

  • Only check files that were added or modified
  • Get an 'annotation' added to the PR converstation
  • Remove PEP8 speaks if above accomplished

[scaramallion]

With pynetdicom I tried out building the docs using github actions but found the extra steps of having to download the artifacts and view locally was too annoying and ended up switching back.

[SimonBiggs]

Was that pymedphys artifacts making it difficult?

[darcymason]

With pynetdicom I tried out building the docs using github actions but found the extra steps of having to download the artifacts and view locally was too annoying and ended up switching back.

I didn't try it but delayed implementing for this very reason. They were supposed to make viewing online available for 'actions' builds relatively soon (or so I thought) after I first posted this. Did you look into it recently?

[scaramallion]

No, this was a couple of months back. It'd make a lot of sense to wait until the online preview is available.

[darcymason]

Tried searching on this again: I thought before I had seen a comment saying it was coming, but now the only thing I can find is this staff comment from Feb '19 which I interpret loosely as 'you have to upload artifacts to some external site for viewing'. And a couple of replies from this May were still complaining about the lack of viewing.

[darcymason]

... Also a thread here (from which one comment points to the above link).

[scaramallion]

The PyPy build would be good, too, I'd like to confirm my refactor of valuerep didn't break pickling.

[mrbean-bremen]

Regarding doc builds on GH actions: I run into the same issue recently, and ended up creating a zipped artifact of the documents, which has to be downloaded and viewed locally - quite inconvenient, but I couldn't find anything better either. So I would just leave it on CircleCI, where it works nicely.

[scaramallion]

Do we want to add a CI workflow to check the typing? It's pretty straight forward

This is the mypy.ini I've been using:

[mypy]
python_version = 3.9
exclude = pydicom/(tests|benchmarks)
files = pydicom/
show_error_codes = True
warn_redundant_casts = True

[scaramallion]

Get an 'annotation' added to the PR converstation

I'm not sure what this means, but the CI improvements are basically done now, right?

[mrbean-bremen]

Get an 'annotation' added to the PR converstation

I'm not sure what this means, but the CI improvements are basically done now, right?

I think this is related to this point from the precursor PR:

flake8 currently running on all files, and first for serious errors (to fail early), and then for all, but errors converted to warning only. However, no PR annotation for this, although there are github actions to post them if we want.

I myself don't care much for Pep8Speaks - I prefer a simple linter command line run (like flake8) that can easily be reproduced locally, like the mypy check. This does not add any annotations to the PR either, of course (I think "annotations" refers to marks in the code similar to the ones by codecov for new uncovered lines).
In the end, I don't think it makes much difference. I'm also perfectly happy with the file name and line number that the linter provides (either one - Pep8Speaks, or flake8, or pylint), not sure if we really need any additional annotations here.

[mrbean-bremen]

I suggest to close this issue. The one remaining issue (adding annotations in the code for findings) is not really worth the effort in my opinion.

[mrbean-bremen]

Having said that, I just had a look at lint-action that seems to support annotations. Maybe we can try it...

[darcymason]

I suggest to close this issue. The one remaining issue (adding annotations in the code for findings) is not really worth the effort in my opinion.

Closing - wouldn't mind a whole look again at CI, but goes beyond the content of this issue.