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

Update keys call in cdscan for Python 3 #399

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

forsyth2
Copy link
Contributor

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

@forsyth2 forsyth2 added bug py3 Python 3.x specific bug labels Jul 10, 2020
@forsyth2 forsyth2 self-assigned this Jul 10, 2020
@forsyth2 forsyth2 marked this pull request as ready for review July 11, 2020 00:12
@forsyth2
Copy link
Contributor Author

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

@forsyth2 forsyth2 requested a review from jasonb5 July 14, 2020 16:56
@jasonb5
Copy link
Contributor

jasonb5 commented Jul 14, 2020

@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
Copy link
Contributor

jasonb5 commented Jul 16, 2020

@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
Copy link
Member

@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
Copy link
Contributor

jasonb5 commented Jul 16, 2020

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

Copy link
Contributor

@jasonb5 jasonb5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@forsyth2
Copy link
Contributor Author

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.

@forsyth2 forsyth2 merged commit 605f9ff into CDAT:master Jul 17, 2020
@forsyth2 forsyth2 deleted the cdscan-python3 branch July 17, 2020 21:59
This was referenced Jul 20, 2020
@jasonb5 jasonb5 added this to the 8.2.1 milestone Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug py3 Python 3.x specific bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError with cdscan
3 participants