DiamondLightSource / cothread

Cooperative Python Threads and EPICS Channel Access bindings
Apache License 2.0
13 stars 9 forks source link

python2 and unicode #4

Closed femanov closed 6 years ago

femanov commented 8 years ago

Hi,

If I pass to python2 version PV name that is u'something' (unicode) to camonitor like this:

chan = catools.camonitor(PVname, NewData, format=catools.FORMAT_TIME)

there will be no events from this monitor without any warning

str, or encoded one will work: chan = catools.camonitor(str(PVname), NewData, format=catools.FORMAT_TIME)

Araneidae commented 8 years ago

This is quite interesting. There are in fact two issues here.

Firstly, if you create a camonitor on an invalid PV then you will indeed get no events with no warning. There is a timeout option which will generate a "disconnected" event, but by default it is disabled. I'm reluctant to change this behaviour because of backwards compatibility, but I can see there is a case to be made.

On the other hand ... passing a unicode string will have actually triggered some really unexpected behaviour. If you look at the return value from camonitor(u'badpv', callback) it will be a list, not a single subscription value! Why is perhaps an exercise for the reader... See what happens if you try caget(u'badpv') instead.

It does seem that the "correct" behaviour would be to convert unicode strings to str, but this does fight slightly with the automatic treatment of lists. Hmm.

femanov commented 8 years ago

you should take into account Python's behavior for strings:

% python Python 2.7.11+ (default, Apr 17 2016, 14:00:29) [GCC 5.3.1 20160413] on linux2 Type "help", "copyright", "credits" or "license" for more information.

u'badPV' == 'badPV'

True

And automatic treatment for this depends on python's version (2 or 3). and it can be a part of making the same module code working with python2 and python3

On Thu, Jun 16, 2016 at 4:54 PM, Michael Abbott notifications@github.com wrote:

This is quite interesting. There are in fact two issues here.

Firstly, if you create a camonitor on an invalid PV then you will indeed get no events with no warning. There is a timeout option which will generate a "disconnected" event, but by default it is disabled. I'm reluctant to change this behaviour because of backwards compatibility, but I can see there is a case to be made.

On the other hand ... passing a unicode string will have actually triggered some really unexpected behaviour. If you look at the return value from camonitor(u'badpv', callback) it will be a list, not a single subscription value! Why is perhaps an exercise for the reader... See what happens if you try caget(u'badpv') instead.

It does seem that the "correct" behaviour would be to convert unicode strings to str, but this does fight slightly with the automatic treatment of lists. Hmm.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dls-controls/cothread/issues/4#issuecomment-226452754, or mute the thread https://github.com/notifications/unsubscribe/AJHSijU83A9jhE3nFWjWEkDZni-mc1lOks5qMStMgaJpZM4I3JAn .

Araneidae commented 8 years ago

Have a look at the offending code (cothread/catools.py), it's not a matter of equality comparison. Actually, I suspect that the simplest solution will be to change the four lines in catools.py which ask "isinstance(pvs, str)" to "isinstance(pvs, (str, unicode))".

Do beware that in Python2 mixing unicode and str values can be fraught with hazards, with conversions happening in unexpected places or not at all. After all, this is why Python3 has completely reworked the semantics of str (with its own headaches elsewhere in the processing chain...)

Araneidae commented 6 years ago

Think this is now fixed: catools on Python2 now treats str and unicode alike.