pre-commit check compliant for github action

[lee1043]

In adaption of pre-commit-hook in the workflow of pmp, codes need to be compliant to the given rules, as discussed in #788 and #788 (comment)

[lee1043]

This PR is a mass diff as @tomvothecoder predicted in his comment, but mostly for reformatting codes to follow style rules (flake8, black, isort, etc). Many thanks to @tomvothecoder for his comments which helped me learn a lot about setting up the github action workflow. There was a steep learning curve but I think I am getting better with it.

• @tomvothecoder, could you please take a look the revised .pre-commit-config.yaml and setup.cfg files to see if they make sense to you?

• @acordonez, could you please help running demos using this branch to see if there is any impact? I expect the result should not be affected by this change. I will also run the demos on my end, but wanted to have additional test from different end to be safer. Please also note that some files under cmec directory has been changed as well, but just about formatting and style.

• @gleckler1, I have reduced flake8 error messages from hundreds to tens, and following two files still have issues and I don't have good ideas how best to fix. Any advice? If that to be too much hassle, I think I can leave them out for now (exclude the 2 files from flake8 test, until they got fixed).

pcmdi_metrics/pcmdi/scripts/pcmdi_compute_climatologies-CMOR.py:

320:43: F821 undefined name 'r'
320:46: F821 undefined name 'i'
320:49: F821 undefined name 'p'

pcmdi_metrics/devel/monsoon_wang/scripts/bar_chart_monsoon_precip_index.py:

218:18: F821 undefined name 'mod_name'
220:32: F821 undefined name 'mod_name'
223:30: F821 undefined name 'mod_name'
226:10: F821 undefined name 'numexpts'
228:23: F821 undefined name 'numexpts'
230:27: F821 undefined name 'numexpts'
231:31: F821 undefined name 'numexpts'
244:27: F821 undefined name 'numexpts'

[tomvothecoder]

Awesome job picking up the QA tools quickly Jiwoo! It's definitely not an easy task.

@tomvothecoder, could you please take a look the revised .pre-commit-config.yaml and setup.cfg files to see if they make sense to you?

Those files look good to me.

@acordonez, could you please help running demos using this branch to see if there is any impact? I expect the result should not be affected by this change. I will also run the demos on my end, but wanted to have additional test from different end to be safer. Please also note that some files under cmec directory has been changed as well, but just about formatting and style.

Yes, the results should be identical since it is simply code and import formatting. It would still be good to validate though.

Note, one potential issue with using isort is if you define env variables (e.g., turning off MPI with cdms2) or set configurations for libraries like matplotlib (e.g., matplotlib.use("Agg") above certain imports. In those cases, you'd have to ignore sorting the lines for those specific imports. I performed a quick search for these cases, but luckily they don't apply.

@gleckler1, I have reduced flake8 error messages from hundreds to tens, and following two files still have issues and I don't have good ideas how best to fix. Any advice? If that to be too much hassle, I think I can leave them out for now (exclude the 2 files from flake8 test, until they got fixed).

That flake8 error F821 undefined name is saying that those variables aren't defined in the current scope, but are being referenced. The fix is to either remove the lines that reference those undefined variables, or add noqa: F821 (not suggested since it leaves broken code in place, unless it is code that is not used in the package build).

---

On a side note, I noticed the GH Actions build job fails on the "Run Tests" step because there are no tests available to run.

============================ no tests ran in 3.17s =============================
Error: Process completed with exit code 5.

https://github.com/PCMDI/pcmdi_metrics/runs/4100746374?check_suite_focus=true#step:5:120

I'm currently exploring a few possible workarounds for this.

[tomvothecoder]

Hey @lee1043, I have addressed the GH Actions build issues in PR #794. Please review the summary of changes (it includes removing mypy for now).

That PR will merge into this PR.

[acordonez]

* @acordonez, could you please help running demos using this branch to see if there is any impact? I expect the result should not be affected by this change. I will also run the demos on my end, but wanted to have additional test from different end to be safer. Please also note that some files under `cmec` directory has been changed as well, but just about formatting and style.

I ran all the notebooks. The two issues I ran into are

1. the MJO, Monsoon Sperber, and Variability Modes drivers can't import VCS
2. I don't have the old style ENSO data anymore, so while I can run the ENSO notebook without error it doesn't generate metrics.
   Let me know if these results are expected or if I should do more testing.

I also did a cmec-driver run using the mean climate driver, which was successful.

[gleckler1]

@acordonez Let's see what JL says about the vcs conflict. We know we need to move away from this but I'm not sure if he is ready for doing that with MJO, monsoon sperber, and variability.

Are we missing some data for the ENSO demo? I thought we were successful in switching to the new style.

[lee1043]

@acordonez Thanks for checking this. While I am refactoring vcs plots (#786), I think it is okay to check output json files to see if there is any noticeable difference. Sorry, I should have clarified this in advance.

By the way could you tell me little bit more what do you mean by no old style ENSO data?

[lee1043]

I think this PR is ready for another round of review. I've fixed broken demo issues in this PR as well, which was triggered by multiple reasons such as (1) some demo input data was lost in web server so recovered, (2) old data (PCMDIobs2) was referred in mean climate and others than the new data (obs4MIPs).

@acordonez would you be able to help running demos on your end when you back? I hope you can run it from scratch, starting from removing previously downloaded demo input data and re-downloading them. They were working okay on my end, but because it is a big PR with many changes, I wanted to double check. Thanks!

[lee1043]

I have double-checked all demos results are consistent to that in the current master. With that, I am merging this PR.

Edited to add: Just in case, I made branch v20211115 as a back up.