fix: fix pyright config

[pjonsson]

Use correct syntax for ignoring
directories, add some ignores
for missing module sources,
and configure the environment
so pyright can find the modules.

This discovered a type error in
conftest.py, so fix that as well.

This gives the expected output:

$ uv run pyright
0 errors, 0 warnings, 0 informations

[pjonsson]

I don't have any experience with mixing Python with other languages and type checking, but pyright by default has stubPath set to typings, so under that directory it expects to find directories for each module with in-house type stubs.

I'm guessing it doesn't expect the stubPath to point to the directory where the source is stored, and that is why it can't figure out the remaining imports, but I'm not sure of that.

I also don't understand how to change the PR title so the check doesn't say Resource not accessible by integration, feel free to edit the title the way you want it.

Edit: I believe it's easiest to just ignore reportMissingModuleSource for the _obstore imports, there isn't supposed to be a Python source file for that module.

[kylebarron]

I also don't understand how to change the PR title so the check doesn't say Resource not accessible by integration, feel free to edit the title the way you want it.

I think this actually means that this line in CI:

obstore/.github/workflows/conventional-commits.yml

Line 4 in be7596f

pull_request:

should be pull_request_target, not pull_request. Would you be able to change that?

[kylebarron]

Edit: I believe it's easiest to just ignore reportMissingModuleSource for the _obstore imports, there isn't supposed to be a Python source file for that module.

Is there a way to do that at the pyproject.toml level like mypy does?

obstore/pyproject.toml

Lines 107 to 109 in be7596f

	[[tool.mypy.overrides]]
	module = ["fsspec.*"]
	ignore_missing_imports = true

[pjonsson]

should be pull_request_target, not pull_request. Would you be able to change that?

I'm pretty sure that change needs to be merged to main before this PR can go green, so I put that in #508.

I don't understand these parts of the permission model in Github, so from my perspective it's quite possible the consequence of that PR being merged will be that a crocodile eats your bedpost.

[pjonsson]

Edit: I believe it's easiest to just ignore reportMissingModuleSource for the _obstore imports, there isn't supposed to be a Python source file for that module.

Is there a way to do that at the pyproject.toml level like mypy does?

Sure, it's just to chuck in reportMissingModuleSource = false in the pyright section of pyproject.toml. Do you want me to put it there and remove the ignores that I added in this PR? (That will disable the check for all sources though, but I assume you already realize the tradeoffs with that.)

[kylebarron]

Edit: I believe it's easiest to just ignore reportMissingModuleSource for the _obstore imports, there isn't supposed to be a Python source file for that module.

Is there a way to do that at the pyproject.toml level like mypy does?

Sure, it's just to chuck in reportMissingModuleSource = false in the pyright section of pyproject.toml. Do you want me to put it there and remove the ignores that I added in this PR? (That will disable the check for all sources though, but I assume you already realize the tradeoffs with that.)

I was hoping that pyright had a per-module way to allow missing type hints. E.g. in

obstore/pyproject.toml

Lines 107 to 109 in be7596f

	[[tool.mypy.overrides]]
	module = ["fsspec.*"]
	ignore_missing_imports = true

mypy allows missing type hints only for the fsspec module. Does pyright not have a per-module config like that?

[kylebarron]

Looks like CI is working now after #509!

[pjonsson]

mypy allows missing type hints only for the fsspec module. Does pyright not have a per-module config like that?

We're deep into the weeds of Pyright now and I'm just a regular user, so I don't know. If you look towards the end of issue 4059 in the Pyright repository, I get the impression they have sliced the problem in a different direction.

I tested defining an environment that set _obstore.pyi as its root and disabled the warning, but that still warned on the imports in __init__.py. I also tried adding a comment at the top of _obstore.pyi that disabled the warning, but it seems the warning needs to be disabled for the importing module.

If one needs to disable a warning for the entire importing module, I would recommend having individual ignores on the lines that need them instead so the rest of the module can benefit from those warnings. The number of imports needing the ignores is small, and the problem appears when type checking obstore itself, I don't think users of obstore will get the warnings. Additionally, the Pyright ignore annotations are not recognized by MyPy and other type checkers, so it will just be a regular comment to those.

[kylebarron]

We're deep into the weeds of Pyright now and I'm just a regular user, so I don't know.

This is fine as-is; I was just wondering if there was an easy config setting like what mypy provides.

Thanks!

[pjonsson]

@kylebarron do you want me to make a PR that reinstates the pyright check in CI?

[kylebarron]

Yes, that would be great; thank you!