Add type checking using Pyrefly

[sydduckworth]

Description

• Added Pyrefly configuration to pyproject.toml
• Added Pyrefly baseline file as .pyrefly-baseline.json to suppress existing typing errors
• Updated Ruff config to enable TC type-checking lints and future-annotations option
• Added type-checking job to Github actions

[braingram]

Can we add this as a pre-commit hook?

[zacharyburnett]

Can we add this as a pre-commit hook?

should this be a pre-commit hook? Having it as a github CI is great, and the user can also install pyrefly and have the language server running with their editor if they choose, but maybe it's too heavy for pre-commit?

[sydduckworth]

My opinion is that type-checking should be treated like the test suite and shouldn't go into precommit.
There are also some practical problems with adding it to precommit:

Precommit runs hooks in an isolated environment, but type checkers need to import project dependencies.
You can configure the type checker to ignore unresolved dependencies, but then the result you get from precommit no longer matches the result from CI.

Alternatively you can have precommit use the system version of the type-checker but then you're requiring users to not only have the tool installed but also have it available in the shell where they're running git commit. This would break VSCode's integrated Git support and probably most editors.
This also breaks my workflow because I use uv for project management so I don't have my virtualenv sourced in my shell, I just use uv run, which precommit obviously can't detect.

[braingram]

What benefit does this type hint provide?

[sydduckworth]

It widens the value type to allow the assignment on the next line to succeed.
In newer versions of Python/numpy its able to infer that slicing into a 1D array produces a 1D array but in older versions it seems like it assumes that slicing into an array produces an array with arbitrary dimensions.

I could have made the value type more specific but wading into the depths of the numpy type system didn't seem worthwhile for something only used in a test case.

[braingram]

Thanks. I should have been more specific. I understand what this does for the type checker but how does it provide benefit to us or our users? Since this is a local variable inside a test function it doesn't provide any benefit to users. For developers I also don't see how it provides any benefit.

I'm also surprised that pyrefly is checking this line since the function is untyped but I guess that's because by default it will check untyped functions due to check-unannoted-defs being enabled. It seems preferable at this point (given that there are no type hints) to start with that disabled. To test this out locally I:

• disabled check-unannotated-defs
• removed the baseline file
• ran pyrefly check

The errors are of 2 types:

• invalid-inheritance, inconsistent-inheritance for a number of classes: TaggedDict, TaggedList, TaggedStr, AsdfObject, the various loaders and dumpers in yamlutil
• 2 errors for the module patching in asdf.util

We could address these by:

• adding ignore comments for the inheritance errors (and opening a follow-up issue on addressing them)
• the errors for asdf.util are for not handling possible None if urllib.parse isn't found. That's a pretty terminal error if a module from the standard library is missing so I'd be ok with either ignoring the errors, updating the code to handle None (and adding a test where urllib.parse is missing) or some other solution.

What do you think about that approach instead of using the baseline file?

[braingram]

Suggested change

	tree: dict[str, Any] = {"a": np.zeros(3), "b": np.ones(10)}
	tree: dict = {"a": np.zeros(3), "b": np.ones(10)}

[braingram]

LGTM. One nitpick on the type hint in the test. We should update the branch protections once this is merged.