search

marbl-ecosys / MARBL

Marine Biogeochemistry Library
https://marbl-ecosys.github.io
Other
14 stars 25 forks source link

Fix CI (again) #415

Closed mnlevy1981 closed 1 year ago

mnlevy1981 commented 1 year ago

Github Actions were failing, I believe due to reliance on old software. Besides updating the specific actions/checkout (from v2 to v3) and actions/setup-python (from v2 to v4), I also updated python from 3.7 to 3.9. I tried going all the way to 3.11, but there is a known issue with a dependency of the old version of pylint we are using, so I tried 3.10... that got past the pylint test but couldn't build the documentation (presumably an issue with the old version of Sphinx we rely on).

So this PR lets a pretty fragile CI setup run again, but I will open an issue ticket about updating both the code we rely on for documentation / testing and also the code we are actually running (with python 3.9, pylint flagged a line of code that was introduced to allow us to support both python 2 and python 3... do we want to maintain py2 compliance? For how long?)

mnlevy1981 commented 1 year ago

I don't think it is explicitly mentioned anywhere in the commit log, but I removed several pylint: disable=C0103 lines from code_consistency.py because that is the code for invalid-name and we are disabling that check in the pylintrc file. netcdf_comparison.py disabled the same check, but used the full name instead of message code (# pylint: disable=invalid-name), and the commit log message for that change is clear.

klindsay28 commented 1 year ago

Regarding py2, my initial thought is that we should support py2 as long as CESM is. In particular, if CESM is expected to work with py2, I think it would be bad for us to not work with py2. I can't think of another reason to support py2, so if CESM drops support for py2, my opinion is that we can drop it also. My 2 cents...

mnlevy1981 commented 1 year ago

Regarding py2, my initial thought is that we should support py2 as long as CESM is. In particular, if CESM is expected to work with py2, I think it would be bad for us to not work with py2.

This seems like a good rule of thumb. I think CESM has dropped py2 support, but I'll verify before doing anything drastic.

I definitely don't plan on changing anything with our scripts until the CI breaks again, but this is the second time CI has broken in the last nine months I'm not sure how long to expect the move from 3.7 -> 3.9 to buy us... we will need to change the setup of the documentation to move to 3.10 and then update pylint to get to 3.11 (or beyond).