percival-detector / percivalui

The Percival detector python user interface
0 stars 2 forks source link

Py3 #129

Closed wnichols1 closed 2 years ago

wnichols1 commented 3 years ago

This is the famous upgrade to python3. Probably need a few more changes and a squash before it's ready to go in.

wnichols1 commented 3 years ago

Implicit in this is a different ordering to the dictionaries because py3 orders them differently. Is this important?

wnichols1 commented 3 years ago

do we want to use a postfix .py3 for these files?

GDYendell commented 3 years ago

do we want to use a postfix .py3 for these files?

I would say no. I have never seen that as a convention.

wnichols1 commented 3 years ago

AM says that all the files in LookAtFLast/ are py3 already.

wnichols1 commented 3 years ago

I had big problems with requirements.txt. The issue is whether to write == or >= when specifying a version. I notice that odin-data uses >= which is better. I think I will change my == to >=, but I put == there so that I could run it on a specific version.

wnichols1 commented 3 years ago

There is also the issue of the modules requests and pyserial. I use them on my button-control program which is optional and not strictly Percival, but the code is in user_scripts. Do I put those dependencies into requirements.txt for convenience?

GDYendell commented 3 years ago

You could do. You could also make a dev_requirements.txt for those extra bits. Or you could add a dev set of requirements here https://github.com/percival-detector/percivalui/blob/a00e912872f9c04b93ead8d041cc5b589d023f98/setup.py#L66 like the test entry.

wnichols1 commented 3 years ago

I dont understand where you are meant to list dependencies - in setup.py or in requirements.txt?

wnichols1 commented 3 years ago

have you checked the versions specified are python3?

GDYendell commented 3 years ago

I think setup.py is better. Installing from requirements.txt is an extra step.

wnichols1 commented 3 years ago

Here is an example demonstrating something that now breaks in py3:

lst = [10,11,12];

for a in range(0, 7): e = (a/3); print(lst[e]);

This worries me because it's not something that the update-tool can notice.

GDYendell commented 3 years ago

That is a shame. What tool are you using to convert it?

I am not sure what the code is doing, but it might be clearer to use divmod and ignore the remainder

descriptive_name_for_what_this_represents, _ = divmod(i, 36)

or just use integer division

i // 36

wnichols1 commented 2 years ago

right well, with two approvals I think I'm going to squash the commits and merge it to master.