Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI (again) #415

Merged
merged 4 commits into from
Feb 14, 2023
Merged

Fix CI (again) #415

merged 4 commits into from
Feb 14, 2023

Conversation

mnlevy1981
Copy link
Collaborator

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?)

Move from python 3.7 -> 3.11, and update to v3 of actions/checkout and v4 of
actions/setup-python
Deprecated github action to use 3.10 instead of 3.11, and then updated
code_consistency.py to account for new pylint warning about formatting
u-strings
3.10 was fine with pylint, but not with sphinx
We do not need to explicitly disable invalid-name in the .py files because that
is handled in pylintrc
@mnlevy1981
Copy link
Collaborator Author

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.

@mnlevy1981 mnlevy1981 merged commit c23d407 into marbl-ecosys:development Feb 14, 2023
@klindsay28
Copy link
Collaborator

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
Copy link
Collaborator Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants