Import CFA from cfdm #842

davidhassell · 2025-01-29T14:24:36Z

Fixes #841

Remove core CFA functionality, importing it from cfdm instead.

Aims to not change the API, other than as required by the new cfdm.

sadielbartholomew · 2025-02-20T15:52:48Z

Just resolved a trivial (changelog items) merge conflict on the branch, so that I can finalise my review without that muddying the waters.

sadielbartholomew

All good, with only minor comments and suggestions raised.

Further, one general issue I noticed - though it isn't clear if it relates to the PR duo moving CFA across to cfdm, plausibly it came through from the active storage PRs a while back - is that there's some environment issue with h5netcdf relating to reading for URLS, we require h5py>=3.10.0 which I would have thought would be acceptable for this but it seems to insist on using h5pyd such that I see this error in the test suite:

======================================================================
ERROR: test_read_url (__main__.read_writeTest.test_read_url)
Test reading urls.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slb93/git-repos/cf-python/cf/test/test_read_write.py", line 866, in test_read_url
    f = cf.read(remote)
        ^^^^^^^^^^^^^^^
  File "/home/slb93/git-repos/cfdm/cfdm/decorators.py", line 171, in verbose_override_wrapper
    return method_with_verbose_kwarg(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/slb93/git-repos/cf-python/cf/read_write/read.py", line 777, in __new__
    raise DatasetTypeError(f"\n{error}")
cfdm.read_write.exceptions.DatasetTypeError: 
Can't interpret http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc as a netCDF dataset with any of the netCDF backends ('h5netcdf', 'netCDF4'):

h5netcdf:
ImportError: No module named 'h5pyd'. h5pyd is required for opening urls: http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc

netCDF4:
OSError: [Errno -68] NetCDF: I/O failure: 'http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc'
Can't interpret http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc as a PP or UM dataset

Do you see this? Any ideas? (Happy to push it to a separate issue, though.)

cf/data/array/mixin/activestoragemixin.py

sadielbartholomew · 2025-02-21T13:12:45Z

cf/functions.py

+def dirname(path, normalise=False, uri=None, isdir=False, sep=False):
+    return cfdm.dirname(
+        path, normalise=normalise, uri=uri, isdir=isdir, sep=sep
+    )


Did you consider using functools.partial (which if you take my suggestion would naturally need to be imported in the module), like so? It is more concise and direct than wrapping (though I'm not sure if our docstring rewriter would work for it, maybe that was a reason not to use it?).

Suggested change

def dirname(path, normalise=False, uri=None, isdir=False, sep=False):

return cfdm.dirname(

path, normalise=normalise, uri=uri, isdir=isdir, sep=sep

)

dirname = partial(cfdm.dirname, normalise=False, uri=None, isdir=False, sep=False)

Nice idea, but I don't it will work in this context. The best we could do is dirname = partial(cfdm.dirname) (if we specify the kwargs as None we can't set them positionally), and in that case the signature and name is hidden (as functools.partial(<function dirname at 0x7f8a6c2c4e00>)).

Better still would be to delete it altogether :). It's not used any more! We could deprecate it with a "use cfdm.dirname instead message". One for another PR, perhaps.

sadielbartholomew · 2025-02-21T13:13:59Z

cf/functions.py

+def abspath(path, uri=None):
+    return cfdm.abspath(path, uri=uri)


See comment below RE possible use of functools.partial, in this case:

Suggested change

def abspath(path, uri=None):

return cfdm.abspath(path, uri=uri)

abspath = partial(cfdm.abspath, uri=None)

See comment #842 (comment)

cf/read_write/um/umread.py

cf/aggregate.py

cf/data/data.py

Co-authored-by: Sadie L. Bartholomew <[email protected]>

davidhassell · 2025-02-24T08:07:00Z

here's some environment issue with h5netcdf relating to reading for URLS, we require h5py>=3.10.0

we bumped cfdm to h5py>=3.12.0, and should do so here, too (d800005). Does that solve the error you were finding?

Co-authored-by: Sadie L. Bartholomew <[email protected]>

sadielbartholomew · 2025-02-24T13:06:36Z

we bumped cfdm to h5py>=3.12.0, and should do so here, too (d800005). Does that solve the error you were finding?

I was already using 3.12.0 so that's the error emerging from that. I tried updating to 3.13.0 they released last week to see if that fixed this, but it didn't. Do you see the same error? Shall we bump this to another issue?

davidhassell · 2025-02-24T14:10:43Z

Oh! I'm using 3.12.1, and it all works fine for me ...

>>> import cf
>>> cf.environment(paths=False)
Platform: Linux-5.15.0-131-generic-x86_64-with-glibc2.35
HDF5 library: 1.14.2
netcdf library: 4.9.4-development
udunits2 library: libudunits2.so.0
esmpy/ESMF: 8.7.0
Python: 3.12.8
dask: 2024.7.1
netCDF4: 1.7.2
h5netcdf: 1.5.0
h5py: 3.12.1
s3fs: 2024.12.0
psutil: 6.1.1
packaging: 24.1
numpy: 1.26.4
scipy: 1.15.1
matplotlib: 3.10.0
cftime: 1.6.4.post1
cfunits: 3.3.7
cfplot: not available
cfdm: 1.12.0.0
cf: 3.16.3

sadielbartholomew · 2025-02-24T15:24:06Z

Thanks for the clarification. OK, so I've investigated the environment. Mine, when seeing the error, was (ignore the paths, this is from the test suite print-out):

Run date: 2025-02-24 12:56:56.118878
Platform: Linux-6.6.65-1-MANJARO-x86_64-with-glibc2.40 
HDF5 library: 1.14.2 
netcdf library: 4.9.3-development 
udunits2 library: /home/slb93/miniconda3/envs/cf-env-312/lib/libudunits2.so.0 
esmpy/ESMF: 8.4.2 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/esmpy/__init__.py
Python: 3.12.0 /home/slb93/miniconda3/envs/cf-env-312/bin/python
dask: 2024.7.1 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/dask/__init__.py
netCDF4: 1.7.1 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/netCDF4/__init__.py
h5netcdf: 1.3.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/h5netcdf/__init__.py
h5py: 3.12.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/h5py/__init__.py
s3fs: 2024.6.1 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/s3fs/__init__.py
psutil: 5.9.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/psutil/__init__.py
packaging: 24.1 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/packaging/__init__.py
numpy: 1.26.4 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/numpy/__init__.py
scipy: 1.14.0 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/scipy/__init__.py
matplotlib: 3.9.2 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/matplotlib/__init__.py
cftime: 1.6.4 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/cftime/__init__.py
cfunits: 3.3.7 /home/slb93/miniconda3/envs/cf-env-312/lib/python3.12/site-packages/cfunits/__init__.py
cfplot: 3.3.0 /home/slb93/git-repos/cf-plot/cfplot/__init__.py
cfdm: 1.12.0.0 /home/slb93/git-repos/cfdm/cfdm/__init__.py
cf: 3.16.3 /home/slb93/git-repos/cf-python/cf/__init__.py

and I bumped the main candidate, h5netcdf, to the same version as you but it didn't stop the error. However, when I bumped netcdf4 to 1.7.2 I no longer saw it - but that's bypassing the issue really since it would pass via the alternative netCDF4 option to the h5netcdf route, i.e. no longer hit this for the alternative backend case:

netCDF4:
OSError: [Errno -68] NetCDF: I/O failure: 'http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc'
Can't interpret http:///psl.noaa.gov/thredds/dodsC/Datasets/cru/crutem5/Monthlies/air.mon.anom.nobs.nc as a PP or UM dataset

since it looks like they fixed their own issue in that respect between versions (see Unidata/netcdf4-python#1335).

So, I think with your environment as quoted above, the h5-related issue is still there, but because it works with the netcdf4 route for the netCDF backend it is obscured. There is still an underlying issue - but let's bump it to its own GH Issue?

davidhassell · 2025-02-24T15:44:54Z

OK - thanks for tracking it down! We need to bump to netCDF4==1.7.2 for numpy2 (PR imminent), so could just leave it for that ...

sadielbartholomew

Thanks for addressing the minor feedback, which has all been covered now. Though I still have one test failure in test_docstring, it will disappear when this gets merged into main due to NCAS-CMS/cfdm#322.

Please merge when ready.

davidhassell · 2025-02-25T11:41:16Z

w00t.

davidhassell added 30 commits November 7, 2024 17:05

dev

ce39fd5

dev

3712766

dev

fc0564e

dev

bf07d57

dev

68a61c7

dev

df5d574

dev

5a32001

dev

5932aea

dev

b4bd7c2

dev

2bd687a

dev

eb3f63f

dev

acefc45

dev

c594c75

dev

e7cac6d

dev

f2675af

dev

88ab94a

dev

6f5d08f

dev

ed664b9

dev

73f03b8

dev

5c0c98e

dev

03444d0

dev

60e6ea0

dev

b7e0937

dev

4a26930

dev

847d946

dev

b86a529

dev

26e2574

dev

bb27a98

dev

ad2e0d0

dev

2b6e171

tidy

c333b62

davidhassell added enhancement New feature or request aggregation Rerlating to metadata-based field and domain aggregation netCDF read Relating to reading netCDF datasets netCDF write Relating to writing netCDF datasets labels Jan 29, 2025

davidhassell added this to the NEXT VERSION milestone Jan 29, 2025

Merge branch 'main' into cfa-in-cfdm

242c57c

davidhassell requested a review from sadielbartholomew January 29, 2025 14:25

davidhassell mentioned this pull request Jan 31, 2025

Move to numpy version 2 #843

Open

davidhassell and others added 4 commits February 7, 2025 16:43

PP unpack

bd5e508

dev

5d282ba

fix up redundant files

62c322d

Merge branch 'main' into cfa-in-cfdm

38ce6d4

sadielbartholomew approved these changes Feb 21, 2025

View reviewed changes

davidhassell and others added 3 commits February 24, 2025 07:37

Fix typos

2c4e00d

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Plurals in message

32a722b

Co-authored-by: Sadie L. Bartholomew <[email protected]>

Remove dead code

09cf4c7

Co-authored-by: Sadie L. Bartholomew <[email protected]>

davidhassell and others added 2 commits February 24, 2025 08:07

h5py>=3.12.0

d800005

Update cf/test/test_docstring.py

7ad74d0

Co-authored-by: Sadie L. Bartholomew <[email protected]>

davidhassell added 2 commits February 24, 2025 14:44

dev

5da6db9

dev

55be346

sadielbartholomew approved these changes Feb 24, 2025

View reviewed changes

davidhassell merged commit e3bc06c into NCAS-CMS:main Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import CFA from cfdm #842

Import CFA from cfdm #842

davidhassell commented Jan 29, 2025

sadielbartholomew commented Feb 20, 2025

sadielbartholomew left a comment •

edited

Loading

sadielbartholomew Feb 21, 2025

davidhassell Feb 24, 2025

sadielbartholomew Feb 21, 2025

davidhassell Feb 24, 2025

davidhassell commented Feb 24, 2025

sadielbartholomew commented Feb 24, 2025

davidhassell commented Feb 24, 2025

sadielbartholomew commented Feb 24, 2025 •

edited

Loading

davidhassell commented Feb 24, 2025

sadielbartholomew left a comment •

edited

Loading

davidhassell commented Feb 25, 2025

		def abspath(path, uri=None):
		return cfdm.abspath(path, uri=uri)

	def abspath(path, uri=None):
	return cfdm.abspath(path, uri=uri)
	abspath = partial(cfdm.abspath, uri=None)

Import CFA from cfdm #842

Import CFA from cfdm #842

Conversation

davidhassell commented Jan 29, 2025

sadielbartholomew commented Feb 20, 2025

sadielbartholomew left a comment • edited Loading

Choose a reason for hiding this comment

sadielbartholomew Feb 21, 2025

Choose a reason for hiding this comment

davidhassell Feb 24, 2025

Choose a reason for hiding this comment

sadielbartholomew Feb 21, 2025

Choose a reason for hiding this comment

davidhassell Feb 24, 2025

Choose a reason for hiding this comment

davidhassell commented Feb 24, 2025

sadielbartholomew commented Feb 24, 2025

davidhassell commented Feb 24, 2025

sadielbartholomew commented Feb 24, 2025 • edited Loading

davidhassell commented Feb 24, 2025

sadielbartholomew left a comment • edited Loading

Choose a reason for hiding this comment

davidhassell commented Feb 25, 2025

sadielbartholomew left a comment •

edited

Loading

sadielbartholomew commented Feb 24, 2025 •

edited

Loading

sadielbartholomew left a comment •

edited

Loading