Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add py.typed for marking the library as type checkable #713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
heckad wants to merge 2 commits into firebase:master
base: master
Choose a base branch
Loading
from heckad:add-py-typed

Conversation

Copy link

@heckad heckad commented Aug 17, 2023

nhairs-lumin, mykolamelnykml, alex-paru, and vinicius507 reacted with heart emoji
Copy link

martasd commented Mar 17, 2025

Hi. Can this PR not be merged?

Copy link
Member

Hi. Can this PR not be merged?

Thanks for surfacing this. I don't think we have added type hints for all the functions, yet. However, since this PR was created we have added type hints to all the new functions. This should be a reasonable change to merge. I will take a look

Copy link

Heya, just bumping this so it doesn't get lost (I figure you're busy).
Would be nice to know if this is blocked by some internal discussions or if you just have a lot on your plate.

Copy link
Contributor

@jonathanedey jonathanedey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @heckad for the contribution!

Copy link

This is bad practice, py.typed should only exist when the package has a type distribution, either a stub file or inline annotations.

heckad reacted with thumbs down emoji

Copy link
Author

heckad commented May 7, 2025
edited
Loading

This is bad practice, py.typed should only exist when the package has a type distribution, either a stub file or inline annotations.

Why? If you add them, mypy will check the typing. Even if there are at least some typings, it’s better than nothing. Now, even existing types won't be checked.

Copy link

ViktorSky commented May 8, 2025
edited
Loading

This is bad practice, py.typed should only exist when the package has a type distribution, either a stub file or inline annotations.

Why? If you add them, mypy will check the typing. Even if there are at least some typings, it’s better than nothing. Now, even existing types won't be checked.

The py.typed file (as defined in PEP 561) serves as an explicit declaration that a package distributes valid type information (via inline annotations or stub files)

Type checkers like mypy, pyright, or pylance interpret py.typed as a promise that the package adheres to PEP 561. If no types exist, users may incorrectly assume the package is "type-safe", while in reality, all imports still resolve to Any.

You can see more details on this topic in the typing specification

Here is an example of a correct implementation of py.typed https://github.com/encode/httpx

Copy link
Author

heckad commented May 8, 2025

This is bad practice, py.typed should only exist when the package has a type distribution, either a stub file or inline annotations.

Why? If you add them, mypy will check the typing. Even if there are at least some typings, it’s better than nothing. Now, even existing types won't be checked.

The py.typed file (as defined in PEP 561) serves as an explicit declaration that a package distributes valid type information (via inline annotations or stub files)

Type checkers like mypy, pyright, or pylance interpret py.typed as a promise that the package adheres to PEP 561. If no types exist, users may incorrectly assume the package is "type-safe", while in reality, all imports still resolve to Any.

You can see more details on this topic in the typing specification

Here is an example of a correct implementation of py.typed https://github.com/encode/httpx

If py.typed is not provided, how do typechers interpret typings in the package? Do they respect them or turn absolutely everything into Any?

Copy link

ViktorSky commented May 8, 2025
edited
Loading

@heckad
If the package contains type annotations, it will infer them, but will display a warning that it is not marked as safe py.typed.
But if you have neither type annotations nor stub files, you should check if an external type distribution.
If they could not be found, since the types cannot be inferred, they will be assigned Any.

Copy link
Author

heckad commented May 8, 2025

@heckad If the package contains type annotations, it will infer them, but will display a warning that it is not marked as safe py.typed.

Please check before writing.

Example:

from firebase_admin.app_check import verify_token
v = verify_token(1)
reveal_type(v)

mypy outputs

t.py:1: error: Skipping analyzing "firebase_admin.app_check": module is installed, but missing library stubs or py.typed marker [import-untyped]
t.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
t.py:4: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)

Event verify_token has signature def verify_token(token: str, app=None) -> Dict[str, Any]:

If I add py.typed file I got

t.py:3: error: Argument 1 to "verify_token" has incompatible type "int"; expected "str" [arg-type]
t.py:4: note: Revealed type is "builtins.dict[builtins.str, Any]"
Found 1 error in 1 file (checked 1 source file)

Mypy: mypy 1.15.0 (compiled: yes)

Copy link

ViktorSky commented May 8, 2025
edited
Loading

@heckad
You're right. When accessing firebase_admin from pip, mypy ignores annotations if they don't contain py.typed.
When it doesn't contain py.typed, mypy doesn't infer annotations directly, although you can force it to do so with --follow-untyped-imports.

But the point is that adding py.typed without type information distribution is not good.

from typing import reveal_type
from firebase_admin.app_check import verify_token
v = verify_token(1)
reveal_type(v)
Without py.typed mark

mypy 1.15

test.py:2: error: Skipping analyzing "firebase_admin.app_check": module is installed, but missing library stubs or py.typed marker [import-untyped]
test.py:2: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
test.py:5: note: Revealed type is "Any"
Found 1 error in 1 file (checked 1 source file)

pyright 1.1.400

d:\Desktop\FBATest\test.py
 d:\Desktop\FBATest\test.py:4:18 - error: Argument of type "Literal[1]" cannot be assigned to parameter "token" of type "str" in function "verify_token"
 "Literal[1]" is not assignable to "str" (reportArgumentType)
 d:\Desktop\FBATest\test.py:5:13 - information: Type of "v" is "Dict[str, Any]"
1 error, 0 warnings, 1 information 
With py.typed mark

mypy 1.15

test.py:4: error: Argument 1 to "verify_token" has incompatible type "int"; expected "str" [arg-type]
test.py:5: note: Revealed type is "builtins.dict[builtins.str, Any]"
Found 1 error in 1 file (checked 1 source file)

pyright 1.1.400

d:\Desktop\FBATest\test.py
 d:\Desktop\FBATest\test.py:4:18 - error: Argument of type "Literal[1]" cannot be assigned to parameter "token" of type "str" in function "verify_token"
 "Literal[1]" is not assignable to "str" (reportArgumentType)
 d:\Desktop\FBATest\test.py:5:13 - information: Type of "v" is "Dict[str, Any]"
1 error, 0 warnings, 1 information 

Here what developers expect is that it returns App, not Any, and also type validation is lost

from typing import reveal_type
from firebase_admin import get_app
reveal_type(get_app)
reveal_type(get_app())

Copy link
Author

heckad commented May 8, 2025

I think you are confusing hot with soft. The problem with Any is solved via the --disallow-untyped-calls flag.
Example

from typing import reveal_type
from firebase_admin import get_app
reveal_type(get_app)
reveal_type(get_app())

mypy's outputs

t.py:4: note: Revealed type is "def (name: Any =) -> Any"
t.py:5: error: Call to untyped function "get_app" in typed context [no-untyped-call]
t.py:5: note: Revealed type is "Any"

But this works only if py.typed was added. If not, the output will be

t.py:2: error: Skipping analyzing "firebase_admin": module is installed, but missing library stubs or py.typed marker [import-untyped]
t.py:2: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
t.py:4: note: Revealed type is "Any"
t.py:5: note: Revealed type is "Any"

My point is that py.typed should be the first step, not the last, because it will encourage programmers to add types to the library, rather than waiting another 5-10 years. Those programmers who don't care will see the same Any as now, and those who use --disallow-untyped-calls (usually enabled by strict mode) will see errors and come to add typings to the library(the goal). And so, with the community's help, it will gradually become as typed as httpx.

ViktorSky, alex-paru, and vinicius507 reacted with thumbs up emoji

Copy link

Can you include the type annotations I'm making?

Copy link
Author

heckad commented May 16, 2025

You can create a pull request and wait for it to be accepted. I am not the maintainer of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanedey jonathanedey jonathanedey approved these changes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /