Skip to content

Add termcolor types to deps#139

Merged
agoscinski merged 1 commit intomainfrom
update-types-deps
May 6, 2025
Merged

Add termcolor types to deps#139
agoscinski merged 1 commit intomainfrom
update-types-deps

Conversation

@agoscinski
Copy link
Collaborator

@agoscinski agoscinski commented Apr 24, 2025

See error in CI of other PRs that do not change any code https://github.com/C2SM/Sirocco/actions/runs/14636678151/job/41069300388

It does not make sense to enforcing typing for it if termcolor does not support it https://github.com/termcolor/termcolor/blob/fd08149187df889da631805ab3651e9265eb8f00/src/termcolor/termcolor.py#L130C12-L130C15 I made a TypeAlias. I think this was a bug always but was not detected till som mypy update.

@agoscinski agoscinski requested a review from DropD April 24, 2025 08:12
@agoscinski agoscinski force-pushed the update-types-deps branch 2 times, most recently from d12107a to feb6c57 Compare April 25, 2025 10:23
@leclairm
Copy link
Contributor

leclairm commented May 5, 2025

The mypy issue is due to a recent change in termcolor from this PR: termcolor/termcolor#97
Can't we just type colors as str then? I mean without a TypeAlias.

@agoscinski
Copy link
Collaborator Author

agoscinski commented May 6, 2025

Yes we can, just tried to respect what the person did by introducing this import of Color. This way you still document that this is a Color type (even though a color type is just str). Whatever you want, I will do it, I just want to merge this.

Replace the `Color` type by `str` as it does not exist in termcolor.
The mypy issue is due to a recent change in termcolor from the PR
termcolor/termcolor#97
@agoscinski agoscinski force-pushed the update-types-deps branch from c40bb1f to 096da1d Compare May 6, 2025 09:52
@leclairm leclairm self-requested a review May 6, 2025 11:33
@agoscinski agoscinski merged commit f570c7e into main May 6, 2025
7 checks passed
@agoscinski agoscinski deleted the update-types-deps branch May 6, 2025 12:04
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.

2 participants