Fix Version/s: None
lsst.pex.config.ChoiceField fails in Python 2 with:
ValueError: ChoiceField's allowed choice variance is of incorrect type str. Expected future.types.newstr.newstr
if created using normal strings for the choices or default. I saw this in noiseReplacer.py when converting meas_base to be Python 3 compatible.
DM-7262 Port meas_base to Python 3
|Assignee||Tim Jenness [ tjenness ]|
|Team||Architecture [ 10304 ]|
That fixed the problem in meas_base. Do you recommend that I undo my workaround in meas_base and make
DM-7262 depend on this ticket?
Actually, I'm surprised it works with your workaround in place given the unicode strings being passed in. I'd prefer it if we backed out the patch as it should work without it now.
Interesting. I had not even tried your fix to pex_config against my patched meas_base code. I just did and it fails, as you predicted. That's a bit disturbing, but in any case I'll undo my workaround in meas_base and make that ticket depend on this one.
Right, pex.config.Config has an autocast function in it that converts str to classic str so that won't be a problem. Everything works on python 3 because there is only one str. If we code like we always have in the past (no unicode) then we'll never trigger the problem. If people start throwing unicode around then we'll have to revisit pex_config. The problem is that it tests that the data type you said you were using matches the data type of the choices you supply. Future str is actually unicode so you fixed it by giving unicode choices. All the code we have is assuming str is the same type as string literal so I made that true in pex_config. In theory I could add an "expected failure" test for python2 but I don't think it gains us anything. A more complete fix of pex_config to properly handle string types will require a bit of thought.
|Reviewers||Russell Owen [ rowen ]|
|Status||To Do [ 10001 ]||In Review [ 10004 ]|
This looks good to me. I verified that it is possible to override Choice entries from the command line (by tweaking pipe_base examples/argumentParser.py to add a choice field) and that the new unit test fails with the old code. The test could be expanded to make a Config and run validate and such on it, but given that the test fails with the old code, it probably suffices.
|Status||In Review [ 10004 ]||Reviewed [ 10101 ]|
|Resolution||Done [ 10000 ]|
|Status||Reviewed [ 10101 ]||Done [ 10002 ]|
Russell Owen I've done a quick fix for the problem. It won't fix all possible combinations of unicode, string literals and future str but let me know if it fixes your particular problem.