cjekel / piecewise_linear_fit_py

fit piecewise linear data for a specified number of line segments
MIT License
289 stars 59 forks source link

add encoding to io.open for windows compat #71

Closed h-vetinari closed 4 years ago

h-vetinari commented 4 years ago

For the conda-forge packaging efforts, we initially went with noarch, which means that only one package gets built, because the package shouldn't depend on any OS- or python-version-specific things (like compilers).

However, to accommodate the update for 2.0.1 in https://github.com/conda-forge/pwlf-feedstock/pull/1 (namely the distinction to use importlib-metadata only on python>=3.8), I had to disable the noarch setting, which meant that then all builds got done separately. This uncovered an error on windows, because README.rst apparently has some non-ASCII signs, and this fails because io.open on windows has a default of encoding=cp1252.

Fortunately, the fix is simple.

cjekel commented 4 years ago

Thanks for catching the issue an the PR! Looks like I messed this up when switching back to rst. I need to release a 2.0.2 patch on pypi for the windows users :)

h-vetinari commented 4 years ago

No problem! :)

cjekel commented 4 years ago

@h-vetinari So did the importlib-metadata made the conda-forge pipeline more complicated? Looking at the other options, I think I can remove importlib-meta requirement and just import pwlf.version in the setup.py file.

h-vetinari commented 4 years ago

@cjekel Everything is automated, so don't worry about the longer runtime. I'd much rather have it like that, then runtime-depend on setuptools. If you can avoid importlib-metadata, there are some small gains in decreasing complexity, but OTOH, we wouldn't have caught the windows bug otherwise. ;-)