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

pep_563 flag creates imports that don't match the usage of the name #313

Open
sternj opened this issue Dec 3, 2023 · 4 comments
Open

pep_563 flag creates imports that don't match the usage of the name #313

sternj opened this issue Dec 3, 2023 · 4 comments
Labels

Comments

@sternj
Copy link

sternj commented Dec 3, 2023

Running monkeytype run NumPyCNN.py; monkeytype apply extracted_conv_ on this gist produces irregular imports, which look like this:

def extracted_conv_(filter_size: int, img: from typing import TYPE_CHECKING

numpy.ndarray,conv_filter: from typing import TYPE_CHECKING

numpy.ndarray,result: from typing import TYPE_CHECKING

numpy.ndarray) -> from typing import TYPE_CHECKING

numpy.ndarray :
@carljm
Copy link
Contributor

carljm commented Dec 17, 2023

I'm not able to reproduce the results you show. Using Python 3.12 and the latest MonkeyType and the code in your gist (after pip install scikit-image), I ran monkeytype run NumPyCNN.py and then monkeytype apply --pep_563 extracted_conv_, after which the first few lines of extracted_conv_.py look like this:

from __future__ import annotations
import skimage.data
import sys
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from numpy import ndarray

def extracted_conv_(filter_size: int, img: numpy.ndarray,conv_filter: numpy.ndarray,result: numpy.ndarray) -> numpy.ndarray:

This is buggy (because the annotations use numpy.ndarray while the import is from numpy import ndarray), but it doesn't look quite as buggy as your report shows.

I'll look into fixing the bug I observe; let me know if you have any further theories about what I should try to reproduce your results. It might be related to the LibCST version; I'm using libcst==1.1.0.

@sternj
Copy link
Author

sternj commented Jan 3, 2024

Confirmed that working with the current release of Monkeytype and libcst==1.1.0 everything's fine on this issue, do you have a tracking issue for the other bug that you observed?

@sternj sternj closed this as completed Jan 3, 2024
@carljm
Copy link
Contributor

carljm commented Jan 3, 2024

I don't, I'm inclined to just reopen this issue and use it, since it's the same repro case (albeit different output.)

When you say "everything's fine on this issue," do you mean that you observe the same output I did (which is not exactly "fine" but it isn't as bad as what you got), or do you mean that you actually observe fully-working output?

@carljm carljm reopened this Jan 3, 2024
@carljm carljm added the bug label Jan 3, 2024
@carljm carljm changed the title pep_563 flag creates syntactically incorrect imports pep_563 flag creates imports that don't match the usage of the name Jan 3, 2024
@sternj
Copy link
Author

sternj commented Jan 3, 2024

I meant that I'm observing the same output as you do, I'm also running into fun issues with ellipses not having qualnames in the serialization phase that I'm trying to get a replication of for you, I have a fork that adds in a branch to account for it but it's a hacky fix for a problem that I can't replicate on another computer.

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

No branches or pull requests

2 participants