search

pasqal-io / Pulser

Library for pulse-level/analog control of neutral atom devices. Emulator with QuTiP.
Apache License 2.0
159 stars 57 forks source link

Explicitly define encoding in open() calls #593

Closed HGSilveri closed 9 months ago

HGSilveri commented 9 months ago

Up until now, we have been using open() without specifying the encoding. However, this is a mistake because open() does not default to utf-8 encoding but rather to the system's default encoder. Since all our device's have utf-8 as the default encoder we never ran into any issues.

However, there are cases where this will be a problem. For example, a Windows user with Chinese character support will have a different default encoder, which means these files will either not be decoded correctly or will fail to be decoded altogether.

The way to fix this is to explicitly specify the encoding, as is done in this hotfix.

HGSilveri commented 9 months ago

These changes look good to me ! However, we also use open in setup.py, conf.py, _version.py and some init.py, should not we also modify them as well ? Won't the installation on a windows with chinese character support fail if we don't modify them ?

I thought about that... I'm pretty sure the installation of the built package (ie from PyPI) won't fail because none of the open() calls occur there. For developers, it's possible that it will affect them, yes. But I'm afraid that there might be other unforeseen issues with mixing development systems, so I thought I'd leave these calls as a sort of flag that tells the developer they are not working in similar system to ours.

Well, it was that and the fact that there are a lot more things to change and I was lazy...

Do you think it's preferable to change everything?

a-corni commented 9 months ago

These changes look good to me ! However, we also use open in setup.py, conf.py, _version.py and some init.py, should not we also modify them as well ? Won't the installation on a windows with chinese character support fail if we don't modify them ?

I thought about that... I'm pretty sure the installation of the built package (ie from PyPI) won't fail because none of the open() calls occur there. For developers, it's possible that it will affect them, yes. But I'm afraid that there might be other unforeseen issues with mixing development systems, so I thought I'd leave these calls as a sort of flag that tells the developer they are not working in similar system to ours.

Well, it was that and the fact that there are a lot more things to change and I was lazy...

Do you think it's preferable to change everything?

Ah yes I had not thought about all of that. Let's go with these changes then !