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

colorama stubs #5108

Merged
merged 5 commits into from
Mar 27, 2021
Merged

colorama stubs #5108

merged 5 commits into from
Mar 27, 2021

Conversation

hatal175
Copy link
Contributor

As I've noticed you usually welcome stubs for pypi packages, I didn't open an issue and just open a pull request as these are new stubs for the colorama package.
I think they are pretty full (not a big package).

Any input is welcome.

@hatal175
Copy link
Contributor Author

hatal175 commented Mar 15, 2021

Um, actually I looked at the code again. There's a class called AnsiCodes. The way it's built, the class has its attributes as int but every specific instance has them as string (converted in init. How would you suggest handling this?

@Akuli
Copy link
Collaborator

Akuli commented Mar 18, 2021

I would add a comment about it and let them be strings, since most people presumably want to use them as strings. @srittau @hauntsaninja any thoughts?

>>> from colorama.ansi import AnsiFore
>>> AnsiFore.CYAN
36
>>> AnsiFore().CYAN
'\x1b[36m'

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I have a few small comments.

For the int/str problem, it looks like users would normally just access the str values, so it makes sense to put those in the types. If it becomes an issue we could use Any instead.

stubs/colorama/METADATA.toml Outdated Show resolved Hide resolved
stubs/colorama/colorama/ansitowin32.pyi Outdated Show resolved Hide resolved
stubs/colorama/colorama/initialise.pyi Outdated Show resolved Hide resolved
@ssbarnea
Copy link
Sponsor Contributor

Did colorama refused to add types directly to it? I kinda prefer to see type hints directly in the libs but even stubs here are far better than nothing, especially for a popular library like colorama.

@hatal175
Copy link
Contributor Author

hatal175 commented Mar 27, 2021 via email

@ssbarnea
Copy link
Sponsor Contributor

@haakenlid Ask, the answer is not always positive but it would make it easier to maintain. A major stopper was so far support for old pythons but as soon a library starts to require >= py36, it makes sense to add type directly to the code.

@hatal175
Copy link
Contributor Author

hatal175 commented Mar 27, 2021

There's this:
tartley/colorama#206
which is not really resolved since early 2019.

I defer to your judgement about this. What should I push for?

@JelleZijlstra
Copy link
Member

It's fine to just add stubs if the library author is not responsive or not willing to add types. In general we want to make it easier to distribute stubs through typeshed.

@JelleZijlstra JelleZijlstra merged commit 577dbe9 into python:master Mar 27, 2021
@hatal175 hatal175 deleted the colorama branch April 6, 2021 22:36
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.

None yet

4 participants