search

CDAT / cdms

8 stars 10 forks source link

Update keys call in cdscan for Python 3 #399

Closed forsyth2 closed 3 years ago

forsyth2 commented 3 years ago

Update calls in cdscan for Python 3. Resolves #397.

forsyth2 commented 3 years ago

@jasonb5 Ready for review. I wrote a script to check for any instances of keys(), values(), or items() not wrapped in list or in a loop. I think these changes should be sufficient, but I haven't run unit tests on it.

jasonb5 commented 3 years ago

@forsyth2 The changes look good but lets get the unit tests ran against this for sanity sake. I'm working on the dev-environment so you can run unit tests locally, let me get that into master and you can update and run the tests.

jasonb5 commented 3 years ago

@forsyth2 You can sync with master and use make dev-environment and make run-tests. Once that's done we can get this merged.

muryanto1 commented 3 years ago

@forsyth2 @jasonb5 I just noticed line 77 and line 79 of the Makefile in master branch needs to be fixed. https://github.com/CDAT/cdms/blob/master/Makefile#L77-L79 - It should be "-c cdat/label/nightly -c conda-forge".

jasonb5 commented 3 years ago

@forsyth2 @jasonb5 I just noticed line 77 and line 79 of the Makefile in master branch needs to be fixed. https://github.com/CDAT/cdms/blob/master/Makefile#L77-L79 - It should be "-c cdat/label/nightly -c conda-forge".

@muryanto1 I think this is fine, the reason for it was in a dev environment i want to start with the latest released version of everything. If we want to change this behavior lets do another PR so we can get this merged and move on with 8.2.1.

forsyth2 commented 3 years ago

I managed to run the tests and got Ran 55 tests, 0 failed (100.00% success). Jason also confirmed the original error (#397) is fixed by this.