peterbrittain / asciimatics

A cross platform package to do curses-like operations, plus higher level APIs and widgets to create text UIs and ASCII art animations
Apache License 2.0
3.64k stars 238 forks source link

change win32 dependence from pypiwin32 to pywin32 #275

Closed drewarnett closed 4 years ago

drewarnett commented 4 years ago

Please consider changing the win32 dependency from pipywin32 to pywin32.

I was researching TUI libraries, especially asciimatics, after learning about it from local peer. Part of that was looking at the dependencies.

The pypiwin32 dependency is declared here: https://github.com/peterbrittain/asciimatics/blob/master/setup.py#L73. This appears to date back to 2015.

Googling this dependency to learn about it, I think it may be preferable to change this to pywin32. Googling showed:

[1] https://mail.python.org/pipermail/python-win32/2016-October/013786.html [2] https://pypi.org/project/pywin32/#files

I suspect pypiwin32 was a workaround for a problem that no longer exists. If that is so, then replace the pypiwin32 dependency in setup.py with pywin32.

One workaround may be to wrangle pip to install asciimatics without installing pypiwin32. (99% sure this can be done, but need to look it up in the pip documentation.)

Related but similarly dated # 111.

Thanks!

peterbrittain commented 4 years ago

Seems like a reasonable change... Want to do the honours?

drewarnett commented 4 years ago

Sure. I can generate a pull request with the pypiwin32/pywin32 replacement in the 4 files it appears in:

I'll try the pip wrangling workaround first per whatever the project has for test.

drewarnett commented 4 years ago

OK if I don't try appveyor first? Seems safe to skip.

Also, looks like appveyor is pointing to a Dec 2014 version "python.exe -m pip install pypiwin32==219". Latest is Feb 2018 per pypi.

Prefer to point to specific version of pywin32 in appveyor.yaml (228) or prefer to not specify (use latest)?

peterbrittain commented 4 years ago

Thanks.

The pinning was to fix a build issue with pywin32. Happy to remove the pinning if it now works on the latest version.

drewarnett commented 4 years ago

Did a bit of pre-implementation testing. Far from thorough, but enough to give confidence.

peterbrittain commented 4 years ago

Looks good. Thanks.