Conversation
1b4a996 to
07f7cb9
Compare
There was a problem hiding this comment.
@alec-bike there are 2 PRs here
Pyright
Also, the experimental
tool.pyrightsection has been simplified (and tested againstpyrightandbasedpyright).
Could you set up pyright to run in CI first, before making changes like this please?
I was working towards that a while back, but there's nothing verifying your claims.
Locally, pyright is complaining about things that are invisible to a reviewer:
Ruff
I don't see any value in removing C901 comments.
Refactoring the functions is a good way to learn about how altair works or just simplify things for future readers 🙂
07f7cb9 to
fc7c6ff
Compare
|
Thanks for the review @dangotbanned.
I've been wondering about the status of pyright (and multiple type checkers) in altair. Is the goal for altair to support both mypy and pyright in CI? I've been running ty for my testing, using a local |
0bac43d to
c32f652
Compare
0d679f9 to
41687d0
Compare
I guess this is a history of the progress I made with Before that, there was a long road to supporting This section of the config was where there were still gaps, but IIRC some test modules were an issue as well: Lines 374 to 378 in 356c78b
I'm very much looking forward to using Using Running |
dangotbanned
left a comment
There was a problem hiding this comment.
Thanks for the changes @alec-bike
I do still have a few more questions though
# Conflicts: # pyproject.toml
212841c to
e84ddc9
Compare

This PR tidies up some ruff rules and comments in
pyproject.toml:increase max-complexity to 15 (allows removal of somerevertednoqa: C901annotations)Also, the experimental
tool.pyrightsection has been simplified (and tested against pyright and basedpyright).Footnotes
ruff preview was turned off in fix: type issues with Chart mark methods. #3936 ↩