NCAS-CMS / cf-python

A CF-compliant Earth Science data analysis library
http://ncas-cms.github.io/cf-python
MIT License
127 stars 19 forks source link

Wierd behaviour by cf-dump #770

Open bnlawrence opened 6 months ago

bnlawrence commented 6 months ago

The output of cf-dump seems to be seeing some sort of escaping and not liking it. Problem manifests in a docker container (you can use the atmbnl/datatools container to see it?

!cfdump mars-era-teaching2023.nc
/opt/conda/envs/datatools/bin/cfdump:14: SyntaxWarning: invalid escape sequence '\-'
  manpage = """\
/opt/conda/envs/datatools/bin/cfdump:108: SyntaxWarning: invalid escape sequence '\ '
  " Manual page cfdump(1)\ ?ltline\ %lt?L/%L.:",
Field: air_pressure_at_mean_sea_level (ncvar%msl)
...

So it doesn't like it, then does the right thing ... so the bug is the spurious warning error message!

Environment:

python
Python 3.12.3 | packaged by conda-forge | (main, Apr 15 2024, 18:38:13) [GCC 12.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import cf
>>> cf.__version__
'3.17.0'
>>> 

(But this is the "special" version using

RUN pip install git+https://github.com/davidhassell/cfdm@h5-read-0
RUN pip install git+https://github.com/davidhassell/cf-python@active-storage-0

)

sadielbartholomew commented 6 months ago

Thanks for reporting, Bryan. I have seen some such warnings recently elsewhere (not from cfa but in cf-python within interactive Python) and from that I believe they only emerge at Python 3.12, or at least much newer versions of Python. They are definitely not emerging in Python 3.8, at least in the separate cases that raise them I observed (which may be the same ultimate origin).

We will investigate. FYI we have some other harmless warnings being raised (see https://github.com/NCAS-CMS/cfdm/issues/293) that we are overdue to get removed from spamming the user. Hopefully I can find some time and fix both of these at once.

davidhassell commented 6 months ago

Hi @bnlawrence, I'm a bit stumped, because that line in cfdump is only executed if you want to look at the man page, i.e. with cfdump -h. As @sadielbartholomew suggests, the only thing we have is that it's a Python 3.12 thing (I'm running 3.11.8 and don't see it). I've looked at the Python 3.12 changelog, but haven't seen a likely cause, yet.

Removing all of the \ from https://github.com/NCAS-CMS/cfdm/blob/main/scripts/cfdump#L15-L108 still works fine at 3.11.8, and I wonder if that will fix things at 3.12 ...

davidhassell commented 6 months ago

Hi @sadielbartholomew, I've tested https://github.com/NCAS-CMS/cfdm/pull/292 (\ removed from cfdump) at Python 3.12.2, and it's fine (no warnings) - could you possibly test it with cfdm at your older version? Thanks!

davidhassell commented 6 months ago

OK, here it is, I think (https://docs.python.org/3/whatsnew/3.12.html#other-language-changes):

A backslash-character pair that is not a valid escape sequence now generates a SyntaxWarning, instead of DeprecationWarning. For example, re.compile("\d+.\d+") now emits a SyntaxWarning ("\d" is an invalid escape sequence, use raw strings for regular expression: re.compile(r"\d+.\d+")). In a future Python version, SyntaxError will eventually be raised, instead of SyntaxWarning.

So, some more general work is probably needed on this.

bnlawrence commented 6 months ago

The weird thing about this is that were was no \ anywhere in my command line ... so I wondered if something was trying to escape the hyphens?

davidhassell commented 6 months ago

I see what you mean, but rest assured that the problem is purely within strings embedded in the library code :)

sadielbartholomew commented 6 months ago

Hi @sadielbartholomew, I've tested https://github.com/NCAS-CMS/cfdm/pull/292 (\ removed from cfdump) at Python 3.12.2, and it's fine (no warnings) - could you possibly test it with cfdm at your older version? Thanks!

Sure, I have just tested this and noted it on the PR (seemed like a more relevant place to submit comments, given it concerns the specific branch, not just the issue at hand).