Lint setup fix

[vigji]

Test fix.

Description

Changes to GitHub actions to fix linting bug. Is this good @adamltyson ? should we handle this differently now that we have the .github repo?

[adamltyson]

Yeah I think it makes more sense to use the actions scripts in brainglobe/actions. I think all that's needed is for tox.ini, .pre-commit-config.yaml and .github/workflows/test_and_deploy.yml to be copied from e.g. imio to this repo.

@brainglobe/ucl-rsdg is there a good way to automate this? We've centalised a lot of the setup, but there are still lots of files in each repo that could get out of date.

[vigji]

But is it centralised? ie for imio, does it happens that if something is changing in brainglobe/.github it is automatically changed also in imio/.github?

[adamltyson]

Not automatically, a new version would need to be released (and the version used in the individual repo may need to be updated), but the changes wouldn't need to be copied & pasted.

[dstansby]

I don't think there's an easy way to share the test/pre-commit configuration files above across repositories unfortunately.

[dstansby]

It's good practice to run this to make sure the latest version of pip is being used - was it causing issues?

[vigji]

I thought it was this line that was entering the endless loop of non-resolvable pip version search, but maybe it was actually one of the docgen packages that I now removed as dev deps. I'll try to put this line back, thanks for the tip!

[vigji]

Actually, I'll just try to emulate the structure of the imio repo as per @adamltyson suggestion

[vigji]

I don't think there's an easy way to share the test/pre-commit configuration files above across repositories unfortunately.

isn't there a way to have a part of a repo that is itself a repo?

[vigji]

Yeah I think it makes more sense to use the actions scripts in brainglobe/actions. I think all that's needed is for tox.ini, .pre-commit-config.yaml and .github/workflows/test_and_deploy.yml to be copied from e.g. imio to this repo.

@brainglobe/ucl-rsdg is there a good way to automate this? We've centalised a lot of the setup, but there are still lots of files in each repo that could get out of date.

There I don't understand the redundancy between the test matrix defined in the workflows .yml and the tox.ini. What of the tox.ini is used where?

[adamltyson]

There I don't understand the redundancy between the test matrix defined in the workflows .yml and the tox.ini. What of the tox.ini is used where?

That I don’t know, @dstansby?

[dstansby]

Do you mean this section: https://github.com/brainglobe/imio/blob/3dac57ee27c8b7f9d0eef2e546319c36597c1e7e/tox.ini#L4-L8 ? It's a hangover from the template tox.ini that napari provide for plugins, and isn't neccessary because tox will automatically select an environment depending on which python version is available, which is set in the github actions config.

[vigji]

Do you mean this section: https://github.com/brainglobe/imio/blob/3dac57ee27c8b7f9d0eef2e546319c36597c1e7e/tox.ini#L4-L8 ? It's a hangover from the template tox.ini that napari provide for plugins, and isn't neccessary because tox will automatically select an environment depending on which python version is available, which is set in the github actions config.

yes, that one! ok, then I can just remove it?

[dstansby]

Yep, you can

[paddyroddy]

Regarding pre-commit I don't think you can share across repos partly because you might have a different configuration across repos. For example, in this one we're using prettier whereas usually we're just using python related ones like this. However, seeing as python stuff would be unrelated to prettier type ones, it wouldn't matter if we used the same pre-commit everywhere. The limitation being that pre-commit doesn't work that way

[vigji]

Ok, this is now fixed, @adamltyson what do you think?
I removed the mypy check, we are not used type hinting at all in this repo, not familiar with it yet and I feel that it makes mypy not so useful here.

@paddyroddy ok, I see! But then this does not standardise much things across repos right? I think that at least for linting we should probably have common tests, and maybe just keep out the (few) repos that won't nee python-related tests?

I feel that it is somehow counterintuitive to have common actions to run tests but then to determine their behavior locally in every repo...(also related to the tox.ini file @dstansby was mentioning)

[adamltyson]

Looks fine to me @vigji!

I feel that it is somehow counterintuitive to have common actions to run tests but then to determine their behavior locally in every repo...(also related to the tox.ini file @dstansby was mentioning)

I agree. If we can't standardise everything with brainglobe/actions, I think we should maybe still have some standard-ish yml files we can keep up to date and copy/paste from. At the moment we're just copying from whatever we think the "best" repo is currently.

[paddyroddy]

Looks fine to me @vigji!

I feel that it is somehow counterintuitive to have common actions to run tests but then to determine their behavior locally in every repo...(also related to the tox.ini file @dstansby was mentioning)

I agree. If we can't standardise everything with brainglobe/actions, I think we should maybe still have some standard-ish yml files we can keep up to date and copy/paste from. At the moment we're just copying from whatever we think the "best" repo is currently.

Yeah I think default config files in brainglobe/actions that you copy-and-paste might be the way to go.

[paddyroddy]

Ok, this is now fixed, @adamltyson what do you think? I removed the mypy check, we are not used type hinting at all in this repo, not familiar with it yet and I feel that it makes mypy not so useful here.

I disagree on your mypy comment. Just because this repo doesn't have much typing now does not mean it won't in the future. Easier to introduce early rather than a very well established repo. Can catch mistakes which will save you time later down the line.

[vigji]

Ok, this is now fixed, @adamltyson what do you think? I removed the mypy check, we are not used type hinting at all in this repo, not familiar with it yet and I feel that it makes mypy not so useful here.

I disagree on your mypy comment. Just because this repo doesn't have much typing now does not mean it won't in the future. Easier to introduce early rather than a very well established repo. Can catch mistakes which will save you time later down the line.

Ok, happy to learn here! Can I ask your help to fix the mypy error that I am getting there? On a python 3.9 (where I am testing this and where tests are running) I get complaints about missing setups in request. The recommendation seems to be to pip install types-requests but it does not fix it in my case, do you know what is happening?

[paddyroddy]

Ok, this is now fixed, @adamltyson what do you think? I removed the mypy check, we are not used type hinting at all in this repo, not familiar with it yet and I feel that it makes mypy not so useful here.

I disagree on your mypy comment. Just because this repo doesn't have much typing now does not mean it won't in the future. Easier to introduce early rather than a very well established repo. Can catch mistakes which will save you time later down the line.

Ok, happy to learn here! Can I ask your help to fix the mypy error that I am getting there? On a python 3.9 (where I am testing this and where tests are running) I get complaints about missing setups in request. The recommendation seems to be to pip install types-requests but it does not fix it in my case, do you know what is happening?

Fixed in #123