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/recursion limit #208

Merged
merged 6 commits into from
Dec 6, 2023
Merged

Conversation

swaradgat19
Copy link
Contributor

fixes #192

@kaczmarj
Copy link
Member

kaczmarj commented Dec 6, 2023

there's a typing error:

wsinfer/patchlib/patch.py:20: error: Argument 1 to "contextmanager" has incompatible type "Callable[[int], None]"; expected "Callable[[int], Iterator[Never]]"  [arg-type]
wsinfer/patchlib/patch.py:21: error: The return type of a generator function should be "Generator" or one of its supertypes  [misc]
wsinfer/patchlib/patch.py:24: error: "setrecursionlimit" does not return a value (it only ever returns None)  [func-returns-value]

try running mypy locally to catch this problem. i believe the return type should be Iterator[None].

from typing import Iterator

# .....

@contextmanager
def temporary_recursion_limit(limit: int) -> Iterator[None]:
    old_limit = sys.getrecursionlimit()
    try:
        yield sys.setrecursionlimit(limit)
    finally:
        sys.setrecursionlimit(old_limit)

@swaradgat19
Copy link
Contributor Author

@kaczmarj I'm able to work around the issue by writing the yield after setting the recursion limit.

@contextmanager
def temporary_recursion_limit(limit: int) -> Iterator[None]:
    old_limit = sys.getrecursionlimit()
    try:
        sys.setrecursionlimit(limit) 
        yield
    finally:
        sys.setrecursionlimit(old_limit)

@kaczmarj
Copy link
Member

kaczmarj commented Dec 6, 2023

@kaczmarj I'm able to work around the issue by writing the yield after setting the recursion limit.

@contextmanager
def temporary_recursion_limit(limit: int) -> Iterator[None]:
    old_limit = sys.getrecursionlimit()
    try:
        sys.setrecursionlimit(limit) 
        yield
    finally:
        sys.setrecursionlimit(old_limit)

looks great!

@swaradgat19 swaradgat19 marked this pull request as ready for review December 6, 2023 18:27
@swaradgat19
Copy link
Contributor Author

@kaczmarj looks good!

@kaczmarj kaczmarj merged commit 2e337ec into SBU-BMI:main Dec 6, 2023
11 checks passed
@kaczmarj
Copy link
Member

kaczmarj commented Dec 6, 2023

thanks @swaradgat19 !

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.

RecursionError: maximum recursion depth exceeded in comparison
2 participants