Decide on preferred set of mypy rules to adopt

[namurphy]

Description of improvement

In the last few months, we have had a flurry of pull requests to add static type checking with mypy to PlasmaPy's continuous integration suite. The current version of mypy.ini defines which mypy rules to ignore on a per-file and per-error basis (#2424), while having the global settings be approximately mypy's strict mode by default. New files would be checked against mypy's strict mode, while existing files have errors be ignored. We should decide on the "final" set of mypy rules to adopt.

Motivation

Enabling static type checking with mypy comes with a bunch of tradeoffs. Having accurate type hint annotations can help with code readability and finding errors, but at the cost of making it hard for making a code contribution. We don't need to enable all of mypy's strict rule set, as it's possible an intermediate level would be most appropriate.

Implementation strategy

We should use this issue as a place to keep track of the rules that are difficult to deal with or that we end up ignoring a lot (using # type: ignore[...]). We can decide later on if the benefits of the rules are worth the tradeoffs.

These rules need not be the same for every subpackage, as suggested in #2438. For example, plasmapy.particles is used throughout the rest of the package, so having more strict typing requirements would be appropriate.

Additional context

Cross-references: #268, #2424, #2432, #2534, #2435, #2438, #2444, #2527

[pheuer]

I forget the error this raises, but:

def fcn(x:None| int = None):
    if x is None:
         x = 0

    y = x + 1

This raises a mypy error because it thinks x can still be None on the last line, and it drives me crazy since this is a pretty common design pattern!

[namurphy]

Huh, the mypy error I get from that is no-untyped-def since there's no return type annotation. mypy is usually good at type narrowing, assuming it can infer what's in the if statement. I'm curious if the error you ran into is happening because there's some dynamical typing happening. 🤔 @pheuer — could you ping me the next time you run into an error like this?

I'm wondering if it may be associated with untyped decorators (like @validate_quantities). Otherwise it's sometimes possible to define a TypeGuard.

Thank you for bringing this up!

[pheuer]

Ah, maybe it is related to decorators? I'll keep an eye out for next time. Here's another pytest specific one

def test_fcn(ex : Exception | None):
    with pytest.raises(ex):
            ...

For some reason mypy is looking for an overloaded function (raising call-overload) for pytest.raises that has Exception as its class, except isn't that just the default (and only?) signature for that function?

[namurphy]

Interesting! I believe the pytest API changed recently so that pytest.raises no longer accepts None, but we get the call-overload error even after removing the | None. (The source file of pytest.raises contains a few signatures of pytest.raises that are decorated with @overload.) In any case, this is a good motivation to ignore call-overload in all test files since the advantages of having the more in-depth type hints don't outweigh the extra complicatedness of having to do all that. Thanks!

[jwreep]

When trying to add particlewise to particle_collections.is_category, the static type check for mypy kept raising issues with this code:

PlasmaPy/src/plasmapy/particles/decorators.py

Lines 525 to 533 in ca92246

	if (
	not self.allow_custom_particles
	and isinstance(particle, ParticleList)
	and any(particle.is_category("custom", particlewise=True)) # type: ignore[arg-type]
	):
	raise InvalidParticleError(
	f"{self.callable_.__name__} does not accept CustomParticle "
	f"or CustomParticle-like inputs."
	)

The "issue" is that the individual Particle class has a method without particlewise as a keyword, so it would only return a bool, which is not iterable. The third condition can only be called, however, given that it passes the check for isinstance(particle, ParticleList), so this cannot be an issue (unless you can somehow pass that second check otherwise?).

This was frustrating trying to debug. The only way I could think was to use nested conditionals, which then ruff complains about. The solution I used was just to tell mypy to ignore it # type: ignore[arg-type].

I don't know if there's any more clever way.