Enable ANN (flake8-annotations) ruleset and fix violations by Sherwin-14 · Pull Request #1371 · earthaccess-dev/earthaccess

Sherwin-14 · 2026-06-27T11:43:48Z

Fixed errors and violations as a result of enabling ANN rulesets. Related to #383.

📚 Documentation preview 📚: https://earthaccess--1371.org.readthedocs.build/en/1371/

github-actions · 2026-06-27T11:43:56Z

👈 Launch a binder notebook on this branch for commit ed3c02f

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit 9af951e

👈 Launch a binder notebook on this branch for commit 3846b5e

👈 Launch a binder notebook on this branch for commit 9dbdaa6

👈 Launch a binder notebook on this branch for commit 2541f40

👈 Launch a binder notebook on this branch for commit 25de6a8

👈 Launch a binder notebook on this branch for commit ee2c8c7

👈 Launch a binder notebook on this branch for commit f2063d5

👈 Launch a binder notebook on this branch for commit 25e320f

👈 Launch a binder notebook on this branch for commit 99214b5

Sherwin-14 · 2026-06-27T13:49:50Z

No clue how to fix the unit test, any ideas?

mfisher87

I think we need to do more in this PR to actually define some of these types instead of using ignores. Many of them appear to be low-hanging fruit at first glance, but I could be wrong. What do you think?

mfisher87 · 2026-06-29T17:48:57Z

    parser: ParserType,
    group: str | None = None,
-) -> Any:
+) -> Any:  # noqa: ANN401


I'd like to fix this one in this PR. I think we can change the ParserType type in earthaccess/virtual/_types.py to be ParserName (and remove | Any there as well), and create a new Parser type which is HDFParser | NetCDF3Parser | KerchunkJSONParser | KerchunkParquetParser. Then this function can accept parser: ParserName | Parser and return Parser. These two new types would need to be kept in sync because Python has no way to dynamically generate string literal types from object names. But it would be better than using Any.

We could then type _parser_map as dict[ParserName, Callable[[], Parser]].

def test_resolve_parser_invalid_string_raises() -> None: """resolve_parser raises ValueError and lists valid names in the message.""" with pytest.raises(ValueError, match="DMRPPParser"): resolve_parser("UnknownParser")

This one starts failing, I can delete this yeah?

I need more detail -- is there a type checker error? Or the test fails? What is the error?

I don't think we should delete the test, I'd have to be convinced :)

Sorry was in a bit of a hurry that day..
and yeah, it's a type checker error.

tests/unit/test_virtual.py:196: error: Argument 1 to "resolve_parser" has incompatible type "Literal['UnknownParser']"; expected "Literal['DMRPPParser', 'HDFParser', 'NetCDF3Parser', 'KerchunkJSONParser', 'KerchunkParquetParser'] | HDFParser | NetCDF3Parser | KerchunkJSONParser | KerchunkParquetParser | DMRPPParser" [arg-type] resolve_parser("UnknownParser")

I'd say we can type ignore that line. We want to test runtime behavior when the defined input type is violated, and the type checker is getting in the way of that. End-users may not be using a type checker so maybe they could get in this situation in real world usage.

mfisher87 · 2026-06-29T17:50:35Z

 def get_urls_for_parser(
    granules: list[earthaccess.DataGranule],
-    parser: Any,
+    parser: Any,  # noqa: ANN401


We could then also use the Parser type here.

mfisher87 · 2026-06-29T17:53:09Z

-    parser: Any,
-    registry: Any,
+    parser: Any,  # noqa: ANN401
+    registry: Any,  # noqa: ANN401


Based on virtualizarr docs, this should be ObjectStoreRegistry (https://virtualizarr.readthedocs.io/en/stable/api/virtualizarr.html)

mfisher87 · 2026-06-29T17:54:34Z



-def __getattr__(name):  # type: ignore
+def __getattr__(name: str):  # noqa: ANN202 # type: ignore


We know the return type here -- Auth | Store | None if I'm reading correctly.

Yeah, but we end up with a cascade of errors if we do that.

~ earthaccess/api.py:530: error: Item Auth? of Auth? | Store? | None? has no attribute "authenticated" [union-attr] if earthaccess.__auth__.authenticated: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Only solution seems to be to add if not isinstance(..., None) where this error shows up.

I read a bit more carefully. Looks like the correct solution is to use typing.overload. If name is "__auth__" then we return an Auth.

@overload def __getattr__(name: Literal["__auth__"]) -> Auth: ... @overload def __getattr__(name: Literal["__store__"]) -> Store: ... def __getattr__(name: Literal["__auth__", "__store__"]) -> Auth | Store: # Or this could be untyped? I don't know what's conventional, but I normally include the type information. The docs show the implementation function as untyped so maybe better to do that. # Actual implementation here

(I think we need to do #1004 and eliminate this singleton pattern before 1.0)

mfisher87 · 2026-06-29T19:00:23Z

        persist: bool = False,  # noqa: FBT001, FBT002
        system: System | None = None,
-    ) -> Any:
+    ) -> Any:  # noqa: ANN401


This should be Auth right? Based on the docstring.

mfisher87 · 2026-06-29T19:03:46Z

        return getattr(proxy, name)

-    def __reduce_ex__(self, protocol: Any) -> Any:
+    def __reduce_ex__(self, protocol: Any) -> Any:  # noqa: ANN401


This looks like it's a tuple of a callable and a tuple of objects of known types. We can type this!

mfisher87 · 2026-06-29T19:04:54Z

    granule: DataGranule,
    auth: Auth,
-    data: Any,
+    data: Any,  # noqa: ANN401


Is this a file path? As a Path or str?

mfisher87 · 2026-06-29T19:05:42Z


 def make_instance(
-    cls: Any,
+    cls: Any,  # noqa: ANN401


I think there is a finite list of classes this could be right?

mfisher87 · 2026-06-29T19:05:56Z

    """Store class to access granules on-prem or in the cloud."""

-    def __init__(self, auth: Any, pre_authorize: bool = False) -> None:  # noqa: FBT001, FBT002
+    def __init__(self, auth: Any, pre_authorize: bool = False) -> None:  # noqa: FBT001, FBT002, ANN401


auth is an Auth, right?

mfisher87 · 2026-06-29T19:06:30Z

+"*.ipynb" = ["T201", "T203", "N803", "RET504", "ERA001", "PLR2004", "PD002", "PD010", "PD011", "EM", "ARG001", "BLE001", "FBT", "ANN001", "ANN002","ANN003", "ANN201", "ANN202", "ANN204","ANN401",]
+"tests/**/*.py" = ["PLR2004", "FBT", "INP001",   "ANN001","ANN002","ANN003", "ANN201", "ANN202","ANN204","ANN401"]
 "scripts/**/*.py" = ["INP001"]
+"stubs/**/*.pyi" = ["ANN401"]


What kind of errors are we seeing with stubs? Our stubs should be well-typed.

Sherwin-14 requested review from chuckwondo and mfisher87 June 27, 2026 11:45

Sherwin-14 force-pushed the ANN branch 11 times, most recently from f2063d5 to ee2c8c7 Compare June 27, 2026 13:36

Enabled ANN ruleset and fixed violations

25e320f

Sherwin-14 force-pushed the ANN branch from ee2c8c7 to 25e320f Compare June 27, 2026 13:48

mfisher87 requested changes Jun 29, 2026

View reviewed changes

Merge branch 'main' into ANN

99214b5



		def __getattr__(name): # type: ignore
		def __getattr__(name: str): # noqa: ANN202 # type: ignore

Uh oh!

Conversation

Sherwin-14 commented Jun 27, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sherwin-14 commented Jun 27, 2026

Uh oh!

mfisher87 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sherwin-14 Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfisher87 Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sherwin-14 commented Jun 27, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Jun 27, 2026 •

edited

Loading

Sherwin-14 Jul 2, 2026 •

edited

Loading

mfisher87 Jul 2, 2026 •

edited

Loading