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

More typing #76

Merged
merged 5 commits into from
Feb 13, 2025
Merged

More typing #76

merged 5 commits into from
Feb 13, 2025

Conversation

elpekenin
Copy link
Contributor

I have just noticed i missed these on my first PR... Sorry about the extra noise

Note that the _question implementation is a bit hacky, but seems to work completely fine with mypy :)

@elpekenin
Copy link
Contributor Author

Just realized args and kwargs can probably be annotated as P.args and P.kwargs, will test and report back tomorrow

@elpekenin
Copy link
Contributor Author

elpekenin commented Feb 4, 2025

Aight, it doesn't seem to work... But i see that sparkline's typing is broken, im taking a look at that

@skullydazed
Copy link
Collaborator

I just added an ignore for that, it's mypy getting picky about a variable that's only used internally to the function. If you rebase it should be working now.

@elpekenin
Copy link
Contributor Author

Just in case, this is ready for review/merge on my end

@elpekenin
Copy link
Contributor Author

Oops, seems like mypy didn't like

# stub
@overload
def question():

def _question(): 

# implementation is not next to the stub
def question()

So... im thinking about ordering like:

  • overloads
  • question
  • _question

@skullydazed
Copy link
Collaborator

I think that can work, it still bundles the code at the end.

If you can't find a way that works with code at the end, we can settle for what does work.

@skullydazed
Copy link
Collaborator

Looks like the latest commit passes. Ready to merge?

@elpekenin
Copy link
Contributor Author

Yeah!

@skullydazed skullydazed merged commit 44d7958 into clueboard:master Feb 13, 2025
16 checks passed
@skullydazed
Copy link
Collaborator

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.

2 participants