Skip to content

Conversation

@randallpittman
Copy link

@randallpittman randallpittman commented Feb 18, 2019

Addresses issue #21.

As a side note, both Python 2 and 3 fail test_constants.py with netcdf4-python v1.4.2, as there's a bug where NETCDF3_CLASSIC files won't mask fill values unless the _FillValue attribute is explicitly set. Adding this line to the CDL in test_constants.py allows all tests to pass (the actual value isn't important for the test):

var1:_FillValue = 9.9692099683868690e+36;

Change list:

  • Basic compatibility fixes: long vs. int, print function, exceptions as
  • fix octal strings to use 0o
  • Fix variable name int_val to long_val
  • integer division
  • string_escape compatibility. Escape coded in CDL files must be valid utf-8 codes.
  • keys() is an iterator in PY3 and cannot be indexed.
  • netcdf NC_CHAR are bytes, not strings
  • basestring compatibility

cdlparser.py -- initial 2/3 compatibility steps

cdlparser.py -- 2/3 compatibility fix octal strings to use 0o

cdlparser.py -- Fix variable name

cdlparser.py -- 2/3 integer division

cdlparser.py -- 2/3 string_escape compatibility. CDL files must be utf-8 compatible.

test_constants.py -- 2/3 compat -- keys() is an iterator in PY3 and cannot be indexed.

test_charvars.py -- 2/3 compat. netcdf NC_CHAR are bytes, not strings

cdlparser.py -- 2/3 basestring compatibility
@randallpittman
Copy link
Author

randallpittman commented Feb 19, 2019

Whoops, I didn't see the other pull request. I didn't try to compare the cdlparser.py code from that PR, but there are some similar changes to the tests, though actually I think that PR has some better improvements to test_constants.py. But I did try to keep this PR narrowly focused to 2/3 compatibility and no pep8'ing!

@randallpittman
Copy link
Author

randallpittman commented Feb 19, 2019

expand_escapes() in Hugo's PR just returned the input string for Py3. I'm still wrapping my mind around that function and its use, so that simpler approach may be better.

Edit: Nevermind, I do understand the function (you can have e.g. \n in CDL and have it turn into a newline), so you do need to do something to convert it. I think my approach works. Hex escape codes have to be valid utf-8 though.

cdlparser.py -- remove unnecessary list() from 2to3

test_constants.py -- Simple fix for netcdf4-python mask problem and make dimname and varname checks more pythonic
… containing either escape codes or unicode characters.
@rockdoc
Copy link
Owner

rockdoc commented Apr 5, 2019

Hi Randall. Apologies for not having looked at this PR until now. For some reason I didn't receive an automatic notification in my mailbox - or if I did it must have gone straight into my junk mailbox and then been auto-deleted (our back-end systems here at work have been migrated recently and the junk mail settings are now all over the shop!)

Anyhows, I'll aim to find time to look at your code changes over the next week or two. As you can tell, I've not been active on GH for some time, so it's going to take me a while to get back up to speed!

self.assertTrue(len(self.dataset.dimensions) == 1)
dimnames = [k for k in self.dataset.dimensions.keys()]
self.assertTrue(dimnames[0] == "dim1")
self.assertTrue('dim1' in self.dataset.dimensions.keys())
Copy link
Owner

Choose a reason for hiding this comment

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

Yep, use of 'in' is the better idiom. Plus, the keys() method is redundant in both the original and new statements.

self.assertTrue(len(self.dataset.variables) == 1)
varnames = [k for k in self.dataset.variables.keys()]
self.assertTrue(varnames[0] == "var1")
self.assertTrue("var1" in self.dataset.variables.keys())
Copy link
Owner

Choose a reason for hiding this comment

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

Yep, use of 'in' is the better idiom. Plus, the keys() method is redundant in both the original and new statements.

@rockdoc
Copy link
Owner

rockdoc commented Apr 10, 2019

@randallpittman Thanks for identifying and implementing the workaround for the NETCDF_CLASSIC bug, and for spotting the handful of occurrences of the now-deprecated except Exception, exc idiom.

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