Conversation
sadielbartholomew
left a comment
There was a problem hiding this comment.
I am reviewing this together with the corresponding cf-python PR NCAS-CMS/cf-python#846, so please wait until I've submitted approval for both to merge anything, but I am happy with the code changes and functionality in cfdm itself hence approving (though I've raised some minor suggestions as usual) and both test suites pass so just doing final checks on the cf-python side now.
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
sadielbartholomew
left a comment
There was a problem hiding this comment.
My approval hasn't changed though I've found a few more means to investigate our numpy 2 compatibility, to be safe, going from the official migration guide.
First, I used ruff as an extra validation (though its reliability is uncertain) that we are numpy 2 compatible on both the cfdm branch here and the cf PR one:
$ ruff check cf/ --select NPY201 ─╯
All checks passed!I've also added np._set_promotion_state("weak_and_warn") to the __init.py__ files to see what warnings might emerge regarding the new type promotion behaviour.
Concerning this cfdm branch, I see a lot of them when running the test suite, though ironically thyey seem to emerge from numpy itself via numpy.ma. I do see one from our code, though:
..
/home/slb93/git-repos/cfdm/cfdm/data/subarray/mixin/pointtopology.py:41: UserWarning: result dtype changed due to the removal of value-based promotion from NumPy. Changed from int64 to int32.
largest_node_id += 1
...so please can you check that the behaviour of that line is safe and as intended in numpy 2. It looks harmless to me, but clearly something triggered the numpy warning so maybe those lines can be reformulated.
Co-authored-by: Sadie L. Bartholomew <sadie.bartholomew@ncas.ac.uk>
Certainly benign, but I've replaced with |
Fixes #318
Notes:
__init__.pyandcore/__init__.pycopykeyword parameter and the__array__interface (https://numpy.org/devdocs/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword)