734 pjg pmpvars docreorg

[gleckler1]

@lee1043 Please modify required variables for ENSO. Also, if you have any tricks to make a cleaner looking json (e.g., no carriage return for each list element) please feel free ;-)

[lee1043]

@gleckler1 This is a great idea to have such JSON file! I made a minor change on the script and JSON but not sure if that was something you were looking for. I also added variables for ENSO.

[gleckler1]

@lee1043 @acordonez the above suggest lots of checks failed but looking on CI it looks OK to me. Maybe one of you can verify and if you think its good to go merge?

[lee1043]

I see the circle ci message that complaining it cannot download https://cdat.llnl.gov/cdat/pmp/sample_data_pr_CMCC.nc (which I can..). I don't think there is any relevance between this error and your PR.

[acordonez]

@gleckler1 Have you been able to run any tests locally? That'd at least catch things like flake8 errors.

[lee1043]

@acordonez great point!

[gleckler1]

@acordonez @lee1043 It's passing flake8 (locally). Have either of you seen before the above "Required statuses must pass before merging" PG not sure what to make of this.

[lee1043]

@gleckler1 I think the "Required statuses must pass before merging" is one of the protection rules that we have set for PMP's master branch, which can be override by people with admin privilege on this repository if needed (not an ideal though). I can do so, but concerned that will result the front page circle ci button to be shown as "Failed" (which already shown as "Failed"... hmm)

[acordonez]

@gleckler1 It looks like we've set all the tests as "required" in the master branch settings. I think we'd have to change the required tests to merge this - maybe just require build-linux and build-macos

[lee1043]

@gleckler1 That's great that flake8 was passed, which usually has been cause of failure from circle ci test.

[gleckler1]

@acordonez @lee1043 "maybe just require build-linux and build-macos" that is very appealing but I'm afraid I don't have a good sense of the implications. Regarding the "Failed" button, I gather we could remove that from the README.md?

[lee1043]

In my understanding the build test only try building (installation) while other "test-linux-3.x"s are try run "tests". I might prefer to keep (some of) those "test" if they work properly, but not a strong opinion.

For the circle CI button in the front readme, I think you are correct that we can remove it.

[acordonez]

@lee1043 @gleckler1 agree that it'd be good to keep some tests as required, but for this PR that means we need to figure out what's going on with the sample data that won't download.