Python 3.12 compatability

[davidhassell]

Fixes #302

Needed for NCAS-CMS/cf-python#770

[sadielbartholomew]

As-is, all good in both Python 3.12 and 3.8 (the two main environments I use for CF tools work, representing our max and min supported Python versions), with no warning emerging from the script running or using the CLI --help.

As you point out, with regards to that specific SyntaxWarning emerging for 3.12, we will likely have other cases to update, both here and in cf-python (e.g. with cfa). I guess you intend to add to this PR for the cfdm updates.

As a side note, is man cfdump meant to work, because it doesn't for me:

man cfa                                                         ─╯
No manual entry for cfa

[davidhassell]

Thanks, Sadie.

I've made a few other related changes, and all the unit tests now pass with no warnings (at 3.12, I haven't tested < 3.12), so I think it's time for review. I've also got a related cf-python PR which I'll put up soon.

man cfdump does not work (couldn't work out how to do that, back in the day), so rendering cfdump -h to look just like a man page was the fall back option.

[sadielbartholomew]

Thanks David. To check before I review, it seems this PR is only to cover to fix the case of a backslash character invalid escape sequence, as per the linked issue and your identification of the 3.12 update that led to it in NCAS-CMS/cf-python#770 (comment), is that true? Or have you checked the list of updates for 3.12 and come to the conclusion that everything is compatible (the PR name suggests that, perhaps)? The test suite passing should obviously be a good sign that we're all good for 3.12 but I wouldn't want to rely on it. So might be wise, if you haven't already, that we read through the list of 'What’s New In Python 3.12' and convince ourselves everything is compatible. Wouldn't have to be in this PR, necessarily.

[davidhassell]

Hi Sadie,

Sorry for not replying to this. I have looked through the 3.12 changes and nothing stood out as needing attention ... and the unit test pass, so I think (hope!) that this is all we need to do for 3.12. Thanks.

[sadielbartholomew]

Hi David, thanks for checking through the Python 3.12 changes for compatibility.

I ran python -W error run_tests.py which fails on any warnings for a conda env with both Python 3.8 and Python 3.12.0, from which I no longer see the warnings which prompted this issue. i.e. #770 from the 3.12 update you identified in NCAS-CMS/cf-python#770 (comment). And the fix seems sensible and as per the request of the warning itself.

However, I do see two different warnings still. The first is from https://github.com/NCAS-CMS/cf-python/issues/625 (which emerges in cfdm so I will transfer it to this repo), which I will take a look at again since I left it behind but we should get it sorted, I feel it is overdue. The second I haven't seen before but probably the message got mixed in with the standard output from the test suite pass/fail report. It is:

======================================================================
ERROR: test_geometry_3 (test_geometry.GeometryTest.test_geometry_3)
Test nodes in a file with no node count variable.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slb93/git-repos/cfdm/cfdm/test/test_geometry.py", line 154, in test_geometry_3
    f = cfdm.read(self.geometry_3_file, verbose=False)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/slb93/git-repos/cfdm/cfdm/read_write/read.py", line 328, in read
    fields = netcdf.read(
             ^^^^^^^^^^^^
  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/cfdm/cfdm/read_write/netcdf/netcdfread.py", line 1543, in read
    geometry_ncvar = self._parse_geometry(
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/home/slb93/git-repos/cfdm/cfdm/read_write/netcdf/netcdfread.py", line 2530, in _parse_geometry
    self._set_ragged_contiguous_parameters(
  File "/home/slb93/git-repos/cfdm/cfdm/read_write/netcdf/netcdfread.py", line 2704, in _set_ragged_contiguous_parameters
    element_dimension_size = int(
                             ^^^^
  File "/home/slb93/git-repos/cfdm/cfdm/data/data.py", line 398, in __int__
    return int(self.array)
           ^^^^^^^^^^^^^^^
DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)

(which appears four times / for four sub-tests). I took the liberty of pushing up the (obvious, I think) fix for that in a new commit 5388324.

Since https://github.com/NCAS-CMS/cf-python/issues/625 is therefore all that remains as to lack of errors and warnings in the test suite for Python 3.12 for cfdm, and it has its own Issue already, I am happy for this to be merged. Please check the commit I pushed here and then merge if you are happy too. Thanks.

[davidhassell]

Hi Sadie - I think that it's fine[*] for us to diverge from numpy's behaviour (i.e. so that int(f) is not always the same as int(f.array)), but I think we need return int(self.array[(0,)*self.ndim] to do this.

[*] for similar reasons that we're happy with an integer index to not drop a dimension

[sadielbartholomew]

Hi David. Oh fair. Feel free of course to from my pushed commit, just write over it or even reset back from it or revert it with git revert. It was useful as an illustration of what could work at least, even if a different approach is safer / more sensible 😄 But overall I would strongly prefer if we sort that DeprecationWarning here in this PR, which promises full Python 3.12 compatibility...

I think that it's fine[*] for us to diverge from numpy's behaviour (i.e. so that int(f) is not always the same as int(f.array)), but I think we need return int(self.array[(0,)*self.ndim] to do this.

I can check to see how numpy does it if useful, they obviously won't want a DeprecationWarning coming through but if that's what they are effectively doing, they'll have to handle it somehow...

[davidhassell]

self.array[(0,)*self.ndim] will always be a 0-dim numpy array, so there should be no more deprecation warnings (and we've got our out TypeError trap on the previous line.

[sadielbartholomew]

self.array[(0,)*self.ndim] will always be a 0-dim numpy array, so there should be no more deprecation warnings (and we've got our out TypeError trap on the previous line.

OK that makes sense. I am happy for you to implement that approach, or I can re-push that update in a new commit if you prefer.

[sadielbartholomew]

Hi @davidhassell, thought I'd bump this since it's been a month or so but this is ready to merge if you just add your adjustment as per #292 (comment) as agreed. I imagine it fell off your radar. (The SyntaxWarnings are quite annoying so good to prevent them.)

[davidhassell]

After some Kafka-esque git related confusion, I have updated Data.__int__, and added unit test to boot.

[sadielbartholomew]

After some Kafka-esque git related confusion

Ah sorry, my pushing to your branch probably caused that difficulty, I now realise. In which case I should have insisted to push the change myself. Next time!

Perfect, I've sanity checked the new commits and they do what we wanted so please merge.