NCAS-CMS / cfunits

A Python interface to UNIDATA’s UDUNITS-2 library with CF extensions:
http://ncas-cms.github.io/cfunits
MIT License
11 stars 8 forks source link

Error reading netCDF files with latest cf-python #12

Closed cjroberts closed 4 years ago

cjroberts commented 4 years ago

Hi,

I am having the following issue when reading files with previously working code after updating cf-python:

Traceback (most recent call last):
  File "/Users/charles/PycharmProjects/common/scripts/cci/makegriddedSSTs.py", line 1015, in <module>
    sst_regridder(args.lon_resolution, args.lat_resolution, time_resolution, year=args.year,
  File "/Users/charles/PycharmProjects/common/scripts/cci/makegriddedSSTs.py", line 169, in __call__
    fl = cf.read(self._filename_groups[0][0])
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cfdm/decorators.py", line 146, in verbose_override_wrapper
    return method_with_verbose_kwarg(self, *args, **kwargs)
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/read_write/read.py", line 655, in read
    field_list = cf_aggregate(field_list, **aggregate_options)
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cfdm/decorators.py", line 146, in verbose_override_wrapper
    return method_with_verbose_kwarg(self, *args, **kwargs)
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/aggregate.py", line 1655, in aggregate
    meta = _Meta(f,
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cfdm/decorators.py", line 146, in verbose_override_wrapper
    return method_with_verbose_kwarg(self, *args, **kwargs)
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/aggregate.py", line 690, in __init__
    self.structural_signature()
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/aggregate.py", line 1151, in structural_signature
    x = [(identity,
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/aggregate.py", line 1153, in <listcomp>
    ('units', tuple([
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cf/aggregate.py", line 1154, in <listcomp>
    u.formatted(definition=True) for u in
  File "/Users/charles/opt/anaconda3/envs/cci/lib/python3.8/site-packages/cfunits/units.py", line 1882, in formatted
    out += ' since ' + self.reftime.strftime()
TypeError: strftime() missing required argument 'format' (pos 1)

I have tried on the command line and receive the same error when reading a different netCDF file.

My environment is:

Platform: macOS-10.15.6-x86_64-i386-64bit
HDF5 library: 1.10.6
netcdf library: 4.7.4
udunits2 library: /Users/charles/opt/anaconda3/envs/cci/bin/../lib/libudunits2.dylib
python: 3.8.3
netCDF4: 1.5.3
cftime: 1.2.0
numpy: 1.19.0
psutil: 5.7.2
scipy: 1.5.1
matplotlib: 3.2.2
ESMF: 8.0.1
cfdm: 1.8.5
cfunits: 3.2.8
cfplot: 3.0.25
cf: 3.5.1

Please would you be able to help?

sadielbartholomew commented 4 years ago

Hi Charles, thanks for raising this.

Firstly I've transferred this over to cfunits Issue Tracker as the bug is emerging from there rather than cf-python itself. At some point a small mistake was made in that codebase as strftime() clearly needs an argument.

with previously working code after updating cf-python:

Interestingly, it seems from git blame that the line in question with the mistake has been in cfunits for ~3 years, so there must have been some change(s) upstream in v.3.5.1 cf-python relative to previous versions which meant the buggy line was run where previously it was not, rather than that line having been introduced for the latest cfunits.

Anyway, I'll push up a fix to the development branch now. Do you have a workaround so that this issue does not get in the way of what you are trying to achieve with cf-python? If not, we are planning to do releases early next week, so there will be a new release set with the bug fixed available then, so you will not have to wait too long.

Thanks again.

cjroberts commented 4 years ago

Hi Sadie,

Thank you for your help.

The Surface Temperature group of NCEO has a web server, which uses cf-python to generate regridded surface temperature (using our own method rather than ESMPy), but we have not upgraded our production version of cf-units yet. So, we can wait until the new release next week before upgrading.

Please could I also ask what the situation is with the conda version of cf-python, which is a few releases behind the PyPI version?

Many thanks.

sadielbartholomew commented 4 years ago

So, we can wait until the new release next week before upgrading.

Excellent. Thanks for you patience.

In terms of a fix, I have resolved the immediate issue (& another one in turn relating to part of the formatted unit being printed without being converted from byte-string to unicode first!) in e4d2b202941c46cb04187dbf722b7e9c7c1ed197, though I haven't tested this much end-to-end to cf-python & I've guessed at the required datetime format to use based on the docstring examples but otherwise am not sure what is intended as a format to return. So I will wait until Monday when I can double check the required/desired format with David before perhaps making some tweaks & marking this as closed. Either way the new releases will contain the final fix.

Please could I also ask what the situation is with the conda version of cf-python, which is a few releases behind the PyPI version?

Sure. You are correct it is a few releases behind.

Overall we have discussed it & are keen & hoping to be able to keep the PyPI & conda latest releases more in sync going forward. We haven't made concrete plans but I imagine we will definitely want to upload the releases we make next week to the ncas conda channel. Since you are another person to request this of late, I shall try to push to get it done next week.

sadielbartholomew commented 4 years ago

Hi, I spoke with David earlier and he thinks the fix in the commit I referenced above is fine & sufficient, so we can consider this closed. The fix will be in the cf-python 3.6.0 out later this week, certainly on PyPI but we'll try to get it out on conda not long afterwards. Thanks.

cjroberts commented 4 years ago

Thank you very much for your help. I will keep an eye out for the release.