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

Deprecations introduced in v0.3.0 are now errors #212

Merged
merged 5 commits into from
Jan 26, 2022
Merged

Conversation

rsokl
Copy link
Contributor

@rsokl rsokl commented Jan 24, 2022

Refer here for list of deprecated patterns, and methods for updating these patterns in your code.

@rsokl rsokl added this to the hydra-zen 0.5.0 milestone Jan 24, 2022
@rsokl rsokl requested a review from jgbos January 24, 2022 19:02
populate_full_signature: bool = False,
hydra_recursive: Optional[bool] = None,
hydra_convert: Optional[Literal["none", "partial", "all"]] = None,
*pos_args,
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you remove the annotations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out that type checkers want the @overload statements to carry all of the annotations, and for the actual implementation to be annotated: https://docs.python.org/3/library/typing.html#typing.overload

@overload
def process(response: None) -> None:
    ...
@overload
def process(response: int) -> tuple[int, str]:
    ...
@overload
def process(response: bytes) -> str:
    ...

def process(response):
    <actual implementation>

pyright was actually complaining, once I removed our deprecation decorators, about mismatched annotations on builds. This was fixed by removing the annotations on the concrete implementation altogether.

Copy link
Contributor Author

@rsokl rsokl Jan 25, 2022

Choose a reason for hiding this comment

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

I should make a comparable change for instantiate, but it isn't creating any issues since the concrete implementation just punts with Any

@rsokl rsokl merged commit 045853c into main Jan 26, 2022
@rsokl rsokl deleted the remove-deprecations branch January 26, 2022 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants