refactor: Add ruff rules, improve type annotations, improve ci performance

[dangotbanned]

Fixes: #3430

• Adds most of the autofixable rules
• Tweak pyproject.toml
• Made some manual fixes, due to comment placement blocking the autofix
• Resolved 2 issues relating to win32 and pyright
• Rerun lint, format, schemagen

Stats from the first round of rules

ruff check . --statistics
Found 313 errors, 293 fixable [*].

Details

91      UP032   [*] Use f-string instead of `format` call
62      EM101   [*] Exception must not use a string literal, assign to variable first
28      EM103   [*] Exception must not use a `.format()` string directly, assign to variable first
24      EM102   [*] Exception must not use an f-string literal, assign to variable first
19      UP008   [*] Use `super()` instead of `super(__class__, self)`
11      RUF002  [ ] Docstring contains ambiguous `–` (EN DASH). Did you mean `-` (HYPHEN-MINUS)?
10      RUF100  [*] Unused `noqa` directive (non-enabled: `C901`)
 8      PIE790  [*] Unnecessary `pass` statement
 8      UP030   [*] Use implicit references for positional format fields
 6      SIM102  [ ] Use a single `if` statement instead of nested `if` statements
 6      SIM118  [*] Use `key in dict` instead of `key in dict.keys()`
 5      SIM108  [*] Use ternary operator `cls = matches[0] if matches else default_class` instead of `if`-`else`-block
 4      SIM117  [ ] Use a single `with` statement with multiple contexts instead of nested `with` statements
 4      SIM300  [*] Yoda conditions are discouraged, use `Version("1.0.0") <= PANDAS_VERSION` instead
 4      RUF005  [*] Consider `(*scope, group_index)` instead of concatenation
 3      SIM910  [*] Use `channel_to_name.get(type_)` instead of `channel_to_name.get(type_, None)`
 3      UP015   [*] Unnecessary open mode parameters
 3      RUF015  [*] Prefer `next(iter(dct["datasets"].values()))` over single element slice
 2      PIE800  [*] Unnecessary spread `**`
 2      PIE810  [*] Call `startswith` once with a `tuple`
 2      SIM105  [ ] Use `contextlib.suppress(AttributeError)` instead of `try`-`except`-`pass`
 2      UP004   [*] Class `ParameterExpression` inherits from `object`
 2      UP027   [*] Replace unpacked list comprehension with a generator expression
 1      PIE807  [*] Prefer `dict` over useless lambda
 1      UP028   [*] Replace `yield` over `for` loop with `yield from`
 1      UP031   [*] Use format specifiers instead of percent format
 1      RUF010  [*] Use explicit conversion flag

Tasks

• Find source of RUF002 characters
  • 10 cases like

  Docstring contains ambiguous ’ (RIGHT SINGLE QUOTATION MARK). Did you mean ``` (GRAVE ACCE...

  • 1 case like

  Docstring contains ambiguous – (EN DASH). Did you mean - (HYPHEN-MINUS)?

• flake8-use-pathlib (PTH) (all manual)
• Preview rules
  • pylint (PLC, E, R, W)
  • refurb (FURB)
• Resolve cases of comments being lost
  • Seems to occur when a comment is nested within an elif/else, but is collapsed after linting
• flake8-type-checking (TCH)/FA100/pyupgrade (UP) manual component
  • For many of these fixes, you first need a from __future__ import annotations declaration at the top of each file.
  • I want to be sure all the maintainers (particularly @binste) understand the impact this can have on any code that relies on typing constructs storing class objects - which would then be "stringified".
  • It would still be a huge positive in simplifying imports, but there may be scenarios where the new behaviour is a source of unexpected issues
  • See PEP 563
  • Also PEP 649 which is due for python=3.14
    suggesting requires further discussion
• Library-specific (mostly manual)
  • flake8-pytest-style (PT)

Future Rule Considerations

• pydocstyle (D)
  • See comment
• Library-specific (mostly manual)
  • NumPy-specific rules (NPY)
  • pandas-vet (PD)

[binste]

This PR grew quite big and I struggle to review it fully so I just did some spotchecks. I think that's fine given the test coverage and linters. I'm sometimes guilty of the same crime and let PRs get quite big... ;) That being said, I really appreciate all the work you put into this, I'm aware that it was probably challenging to keep all PRs in sync, and I think it's a great improvement to the quality of the code base.

I've only added some remarks which should be quick to resolve. The most important one might be the naming of Optional which worries me a bit and I hope we can find a better name there. After that, I'm ok to get this into the code base. Also ok with the __future__ import, thanks for the research on it.

[binste]

I do like the use of a TypeVar for this but I don't know if we should name it Optional as it already has a different meaning which is well known to users. If I read Optional in my IDE, I expect that I can pass None and I might write code for it. What do you think? I can't come up with a better name for it right now...

I really like the literals to reduce the code!

[dangotbanned]

@binste I can't seem to be able to reply to this comment

I do like the use of a TypeVar for this but I don't know if we should name it Optional as it already has a different meaning which is well known to users. If I read Optional in my IDE, I expect that I can pass None and I might write code for it. What do you think? I can't come up with a better name for it right now...

I do agree with this sentiment, and when I first added the alias in f1d9dc2 I used the name _AltOptional for this exact reason.

By the time I added it to this branch, I'd changed my mind and added the reasoning to the commit message:

typing.Optional is deprecated and is no longer needed in altair. The new alias improves the readability of generated signatures, by reducing noise (repeating UndefinedType) and clearly grouping together the options for an argument.
https://typing.readthedocs.io/en/latest/spec/historical.html#union-and-optional

I'm not sure about all IDEs, but for VSCode the "dangling" docstring will show on hover.

Just spotted I need to update the example since https://github.com/microsoft/pylance-release/releases/tag/2024.6.100 added sphinx support.

I expect that I can pass None

That is fair. Although say for the example above, the arguments accepting None are already differentiated.
A user with a type-checker will see an error for None on aggregate, bandPosition - but not bin, field, header.

I am open to an alternative, but the semantics of this alias and typing.Optional are so similar that I felt we could reuse the name - as long as typing.Optional appeared nowhere in altair - to avoid any confusion.

I really like the literals to reduce the code!

Thank you. I was really suprised at how much more readable it made the signatures, especially those with multiple literals.

[binste]

Valid points. I can't come up with a better name either so let's give it a try like this.

I'll merge this in to free up other PRs. Refinements such as updating examples can happen in a follow-up PR.

[dangotbanned]

Just seen this https://github.com/astral-sh/ruff/releases/tag/0.5.0

@binste thanks for reviewing & merging.

I'll continue in another PR as suggested