search

RNO-G / mattak

RNO-G dataformats
1 stars 1 forks source link

import annotations from __future__ to prevent errors on python<3.9 #51

Closed sjoerd-bouma closed 3 months ago

sjoerd-bouma commented 3 months ago

This only solves a problem in voltage_calibration.py currently, but I figured seeing as they are used in the other modules too, it's probably safer to use the 'future' type annotations everywhere. I did verify that this solves the TypeError in #50 on Python 3.7.

Fixes #50

sjoerd-bouma commented 3 months ago

Some more testing - this does unfortunately break python 3.6 support, as the annotations feature was only added to __future__ in python 3.7. On the other hand, voltage_calibration.py definitely doesn't work on python 3.6 as-is either.

fschlueter commented 3 months ago

I am fine with breaking py3.6. @cozzyd whats your opinion on this matter?

cozzyd commented 3 months ago

see the discussion on issue #50. I still hesitate to break python 3.6 compatibility due to EL8 having python3.6 by default and EL8 being used on our server in Greenland (as well as rno-g.uchicago.edu). It's possible to work around this, but sort of annoying. Currently the system-wide ROOT on EL8 (installed from the EPEL repository) is built against python3.6, for example.

At some point in the future, the server in Greenland (and at UChicago) may be upgraded to EL9 (or 10?), but probably not until either I go back to Summit with packages on an external hard drive or the internet there gets a lot faster :).

sjoerd-bouma commented 3 months ago

Closing this in favour of #52 to maintain python 3.6 compatibility.

fschlueter commented 3 months ago

Okay I think Sjoerd found a way to keep 3.6 compatibility. However, in light of the huge performance jumps in newer python versions I wonder if a little pressure to upgrade to newer versions isnt a good idea. Using a newer version on EPL8 should not be difficult?