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

ImportError: cannot import name 'get_terminal_size' from 'click.termui' #2226

Closed
zwitterion opened this issue Mar 28, 2022 · 8 comments
Closed

Comments

@zwitterion
Copy link

Environment:

  • Python version: 3.9 and 3.8
  • Click version: 8.10
@philgooch
Copy link

See also explosion/spaCy#10564

@davidism
Copy link
Member

davidism commented Mar 28, 2022

When reporting an issue, be sure to include a minimal reproducible example, and a description of the problem, as it relates to the library.

The referenced function was previously deprecated, and is now removed. https://werkzeug.palletsprojects.com/en/8.1.x/changes/#version-8-1-0

8.0.0:

click.get_terminal_size() is deprecated and will be removed in 8.1. Use shutil.get_terminal_size() instead. #1736

8.1.0:

Remove previously deprecated code. #2130

  • get_terminal_size is removed, use shutil.get_terminal_size instead.

You should use a tool like pip-tools to pin your dependencies and control when you get updates. Be sure to run tests with deprecation warnings treated as errors so that you get notified of these types of changes early.

@davidism davidism changed the title Spacy is not working with the version 8.10 of click. ImportError: cannot import name 'get_terminal_size' from 'click.termui' Mar 28, 2022
@henryiii
Copy link
Contributor

henryiii commented Mar 30, 2022

X-ref for typer: fastapi/typer#375 (and duplicates), not yet in or released.

Would something like this be acceptable to provide a transition period?

def get_terminal_size(*args, **kwargs):
    msg = "Deprecated, please use shutil.get_teriminal_size instead"
    warnings.warn(msg, FutureWarning, stacklevel=2)
    from shutil import get_terminal_size
    return get_terminal_size(*args, **kwargs)

All typer apps are broken at the moment, and this is not an underscore import, like the one that broke black. Just curious. Actually, guessing from above, something like this was probably there, but probably was using a 'lesser' warning level.

FutureWarning is the correct warning to provide for a removal that is about to happen. According to PEP 0565, there are three levels of deprecation warnings, with FutureWarning being the highest level. Ideally, you step through two or three levels, but the final level should be FutureWarning, since it shows up for everyone.

Also, stating people should lock dependencies does't help if it's a library that's broken (Typer, in this case). If you start a brand new project, pip install typer is broken. Pinning via a typer app via pre-commit is broken, etc. Libraries can't lock their dependencies1, only apps can.

(FYI, pycln in pre-commit is what is broken for me, which depends on typer, which depends on click)

Footnotes

  1. Libraries can lock them in their own CI. But not for users.

@davidism
Copy link
Member

davidism commented Mar 30, 2022

This was already issuing a deprecation warning, documented as such in the changelog, and then documented as removed in the changelog.

Use a tool like pip-tools to pin your dependency tree and control when you get updates. Be sure to run your tests with deprecation warnings treated as errors so that you get notified of these types of changes early.

If typer was testing with warnings as errors, they could have proactively responded to this. If anyone using typer had been testing with warnings as errors, they could have reported this to typer. If projects aren't doing this, no amount of extra work on my part will change that, it will just delay when they finally notice they have a problem with their process.

The typer maintainer is currently at a conference, I'm sure they'll make a release when they get back and have the ability to.

@henryiii
Copy link
Contributor

henryiii commented Mar 30, 2022

Usages like this are breaking:

- repo: https://github.com/hadialqattan/pycln
  rev: "v1.2.5"
  hooks:
  - id: pycln
    args: [--all]

https://scikit-hep.org/developer/style#pycln

There's no way to use pip-tools with something like that. I personally already use locking (via PDM mostly) for applications where it matters to me.

I would really recommend using at least a single release of FutureWarning before removing something without an underscore - obviously, what was done wan't enough for typer, which is a major dependent library. While I like warnings as errors and recommend it (https://scikit-hep.org/developer/pytest#configuring-pytest), it's not that common - many users don't know about it or find it annoying to enable1. Also, due to the despicable practice of capping dependencies (heavily pushed by Poetry), many users are just now removing/updating ^7 caps to ^8, causing them to completely miss the 8.0 release and jump to 8.1. FYI, pytest also removes deprecated items in the .1 releases like click.

I've already spent two days updating 30+ packages pre-commit's to use black 22.3.0, so the 4-5 packages that needed the extra pin added to pycln was a bit annoying, but it is not an issue for me, this was just suggesting something that would be nice for the other users of click via Typer. And the fact our developer guidelines are suggesting something that is broken is also bothersome, thought our cookiecutter already has the current workaround pin. And it was technically these libraries fault for not having warnings enabled, though I like to err on "being nice" over being "correct", and consider it Pythonic.

It's totally up to you, just providing a suggestion that would reduce pain in the ecosystem. Once typer updates, this should go away.

Footnotes

  1. See, for example, https://github.com/scikit-hep/pyhf/pull/1773#pullrequestreview-878643489

@henryiii
Copy link
Contributor

FYI, I mentioned this to Sebastián and he replied "I'm on it!". So this hopefully will be fixed there soon. :)

@henryiii
Copy link
Contributor

Typer 0.4.1 is out. :)

@tiangolo
Copy link
Contributor

Hello! I just released Typer 0.4.1 that should handle this. 🚀 🤓

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants