Skip to content
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

fix: Create py.typed #862

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kongzii
Copy link

@kongzii kongzii commented Aug 13, 2024

Hi, just a small fix: without this, mypy doesn't consider langfuse's typing.

Given the following test file

from langfuse.decorators import observe


@observe()
def test(x: int) -> str:
    return str(x)


y = test(5)
reveal_type(y)

with this change, mypy looks like this:

test.py:9: error: Argument 1 to "test" has incompatible type "str"; expected "int"  [arg-type]
test.py:10: note: Revealed type is "builtins.str"

without this change (with the latest langfuse version), it looks like this:

test.py:10: note: Revealed type is "Any"
Success: no issues found in 1 source file

I see there is some discussion already in langfuse/langfuse#2169, but it got stalled.

It is true that if I just run mypy on this repository, there are many complaints:

main $ mypy . | wc -l                                                                                                                                                                                                                           [10:55:22]
     517

So it would require a big effort to fix the whole library for mypy, but on the other hand, it's a shame that currently, any project that uses @observe is losing type safety, which can be fixed by this change.

As a hot fix, without this change, having this wrapper in our code repository around the langfuse's observer seems like a way to go, but it's not a great solution:

from langfuse.decorators.langfuse_decorator import ( 
    langfuse_context,
    observe as original_observe,
)
from typing import TypeVar, ParamSpec, Optional, Literal, Callable, Iterable, Any

P = ParamSpec("P")
R = TypeVar("R")


def observe(
    name: Optional[str] = None,
    as_type: Optional[Literal["generation"]] = None,
    capture_input: bool = True,
    capture_output: bool = True,
    transform_to_string: Optional[Callable[[Iterable[Any]], str]] = None,
) -> Callable[[Callable[P, R]], Callable[P, R]]:
    casted: Callable[[Callable[P, R]], Callable[P, R]] = original_observe(
        name=name,
        as_type=as_type,
        capture_input=capture_input,
        capture_output=capture_output,
        transform_to_string=transform_to_string,
    )
    return casted

@CLAassistant
Copy link

CLAassistant commented Aug 13, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Disclaimer: Experimental PR review

PR Summary

This pull request adds a py.typed file to enable type checking support for the langfuse library.

  • Added empty langfuse/py.typed file to indicate type checking support
  • Improves type inference for @observe decorator in user projects
  • Enables better static type checking for projects using langfuse
  • Addresses part of the type-related issues discussed in GitHub issue #2169
  • Does not fix all mypy complaints within the langfuse library itself

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants