⛔ Remove pypiwin32 package #1056
Conversation
|
@SimmonsRitchie I think it's ok to remove |
|
@SimmonsRitchie The Cron workflow only runs in |
|
Thank you very much for the review, @LienDang! I really appreciate it. Re: moving Unless you feel strongly about it, I still lean towards just removing this package entirely to make this project easy to install for new contributors. My sense from your second comment is that you're cool with that. |
|
@SimmonsRitchie yes, you are right about moving the lib into dev-dependencies could be tricky to new contributors. But I still wanted to leave a hint for Windows devs to know that they may need to install pypiwin32 for their machine (it's no problem to Git-experienced devs, they can trace back the change (the removal) from Git history 😀). So how about putting a note to the README, or commenting it out under the dev dependencies (there will be no hash for it in the Pipfile.lock)? Anyway I'm ok with merging this PR! It is the cleanest approach 👍 |
|
I like that compromise, @LienDang! I have a forthcoming PR that makes a few tweaks to the readme. I'll include a note, re: pypiwin32. |
Summary
Deletes pypiwin32 from our pipfile and rebuilds our lock file, which appears to solve an issue with our dependencies that was causing our CI github workflow to fail when triggered.
Why are we doing this?
I recently opened a PR (my first for this repo) with relatively small changes and immediately discovered our CI github action was failing. The failure appeared totally unrelated to my changes. The error was occurring just as Pipenv attempted to install packages:
In both python 3.8 and 3.9 virtual environments, I attempted to rebuild our Pipfile.lock file to address this issue but encountered a different but, I suspect, related error:
I tried installing our production dependencies from scratch, one by one, and found that removing pypiwin32 meant I could rebuild our Pipfile.lock successfully. Our CI workflow also could install our dependencies successfully and run normally.
pypiwin32 is a package used by Windows users to allow Python applications to access and use various Windows-specific features and functionalities. It was introduced in #934 in 2019, but unfortunately there are no notes to help explain the exact reason for its introduction.
My suspicion is that the package was, of course, added to provide support for Windows-using contributors of this repo or to make it more accessible to potential contributors. However, my overall impression is that pypiwin32 is now quite outdated and hasn't been maintained (it's last commit was in 2017). My sense is that a different package, pywin32, now provides the preferred Python bindings for Windows and, based on the package's readme, that users handle this as a global install on their machine rather than within their virtual environment on a project-by-project basis. For that reason, I think we can remove the pypiwin32 package and – hopefully – be confident it won't impact the ability of current or future Windows-using contributors to contribute to this repo.
How can I manually test these changes?
pipenv --python 3.9pipenv lockThe command should execute without raising any errors and the Pipfile.lock should be rebuilt.
Are there any smells or added technical debt to note?
As a Mac user, I feel a little uncomfortable removing this package without knowing the full implications for Windows-based devs but it's also unclear to me whether this package was necessarily providing any benefit to them either. My feeling is that right now the priority probably needs to be ensuring we have a functioning CI process and that we should address any complaints/feedback from Windows-users subsequent to this PR being merged.
As an aside, this issue makes me lean towards dockerizing this project to provide reliable cross-platform support. This was discussed in 2019 in #932 and is probably a conversation worth resurrecting.