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

Feature/add_type_information #63

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

9y2070m
Copy link

@9y2070m 9y2070m commented Apr 11, 2023

resolves #56

Solution to add types to package using a stubs file

  • add stubs file (deprecation.pyi)
  • add py.typed marker file to indicate that package has type hints
  • refactor to add top-level module folder to register py.typed and __init__.py

@briancurtin

@9y2070m 9y2070m marked this pull request as ready for review April 11, 2023 05:22
@9y2070m 9y2070m force-pushed the feature/add_type_information branch 4 times, most recently from 0193c91 to f16d2d0 Compare April 11, 2023 08:18
@9y2070m 9y2070m force-pushed the feature/add_type_information branch from f16d2d0 to b502d61 Compare April 11, 2023 08:27
removed_in: Union[str, datetime.date, version.Version, None] = ...,
current_version: Union[str, datetime.date, version.Version, None] = ...,
details: str = ...,
) -> Callable: ...
Copy link

Choose a reason for hiding this comment

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

Consider using a TypeVar for the wrapped function so that the decorator's type hints preserve the function's signature. Pylance (in VS Code) manages to display the signature without this (perhaps due to the implementation's use of @functools.wraps?), but mypy shows Any if I use reveal_type on a deprecated function.

Example: https://github.com/python/typeshed/blob/main/stdlib/contextlib.pyi#L34

_F = TypeVar("_F", bound=Callable[..., Any])

def deprecated(...) -> Callable[[_F], _F]: ...
def fail_if_not_removed(method: _F) -> _F: ...

Comment on lines +14 to +16
deprecated_in: Union[datetime.date, version.Version, None]
removed_in: Union[datetime.date, version.Version, None]
details: Union[datetime.date, version.Version, None]
Copy link

Choose a reason for hiding this comment

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

The examples in the readme pass str for deprecated_in, removed_in, and details. I don't think details is ever a date/version.

Copy link

@nhairs-lumin nhairs-lumin Sep 28, 2023

Choose a reason for hiding this comment

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

It looks like the code supports datetime.date.

Copy link

Choose a reason for hiding this comment

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

Yes, removed_in can be a date. I don't see any indication that the other two fields can be a date.

Based on the code and docstrings, it looks like:

  • deprecated_in: version, str, or None
  • removed_in: date, version, str, or None
  • details: str or None

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.

Add type information (PEP 561)
3 participants