Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only install and import colorama on Windows. #691

Merged
merged 4 commits into from
May 24, 2021

Conversation

hwalinga
Copy link
Contributor

Related: #612

Only install and import colorama on Windows. Importing and init call has no effect on other platforms, so no need to do it on other platforms.

  • I have added an entry to docs/changelog.md

Summary of changes

Just check if on Windows, do import and call init. Also, colorama dep is now listed as a windows specific dep.

Test plan

Tested by running:

pipx list --verbose

@itsayellow
Copy link
Contributor

Does this have a measurable benefit? (Startup time, etc.)

@hwalinga
Copy link
Contributor Author

Barely anything significant. But that is not the reason I took the effort to submit this PR.

To give some background. I use poetry as my dependency manager and poetry will always uninstall colorama because pylint and pytest both list colorama as a Windows only dependency. Breaking pipx every time I use poetry.

You can say this is a poetry bug, or that I just have to install pipx in its own environment, pipx install pipx is what I have now ; ). But listing colorama as a Windows only dependency is just more correct, just as pylint and pytest have done, and just makes sure pipx is a bit more robust on other platforms if there goes anything wrong with colorama.

@itsayellow
Copy link
Contributor

Ah ok, I didn't realize there was this side effect with poetry. Your motivation makes more sense now.

It's too bad, because the whole point of colorama is that it works cross-platform and you don't have to conditionally install on Windows. 😕

src/pipx/colors.py Outdated Show resolved Hide resolved
@hwalinga
Copy link
Contributor Author

Yes, that is true, if other tools didn't specify colorama as Windows only and just ran it unconditionally Poetry would never decided to remove it. It's just that Python dependency management isn't as robust as it can be. Without that problem pipx might have never existed.

Here is the issue from Poetry talking about that:
python-poetry/poetry#3139

src/pipx/colors.py Outdated Show resolved Hide resolved
@itsayellow itsayellow merged commit 64b72f0 into pypa:master May 24, 2021
@jimxliu
Copy link

jimxliu commented Jan 25, 2022

Hello! I came across the following error when I was trying to poetry lock pipx 1.0.0 with colorama 0.4.3 on linux (ubuntu). As stated in this PR, >= 0.4.4 is only required in windows.

  SolverProblemError

  Because pipx (1.0.0) depends on colorama (>=0.4.4)
   and my-app depends on colorama (0.4.3), pipx is forbidden.
  So, because my-app depends on pipx (1.0.0), version solving failed.

And the pyproject.toml looks like this

[tool.poetry.dependencies]
colorama = "0.4.3"
python = "^3.7"
typer = "^=0.4.0"
pipx = "1.0.0"
virtualenv = "^20.8.1"
joblib = "^1.1.0"

system python 3.7

Any thoughts on why it didn't follow the linux requirement? Thank you!

@jimxliu
Copy link

jimxliu commented Jan 25, 2022

I figured out a solution by adding { version = "*", markers = "sys_platform == 'linux'" } to pipx. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants