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

feat/python guess inside ifstat #240

Merged
merged 7 commits into from
Mar 8, 2024

Conversation

blast-hardcheese
Copy link
Collaborator

Why

It's reasonably common for Python libraries to include gate imports behind if, def, and try statements. Let's try adding if/elif/else and see whether this is even a direction we want to proceed with.

What changed

Altered query to reflect the if_statement structure. Extended test to cover this new case.

Test plan

See whether imports gated behind if statements are caught by upm guess

@blast-hardcheese blast-hardcheese requested a review from a team as a code owner March 5, 2024 00:39
@blast-hardcheese blast-hardcheese requested review from airportyh and removed request for a team March 5, 2024 00:39
@blast-hardcheese
Copy link
Collaborator Author

Before this gets merged, we should probably ensure that upm acknowledges poetry dependency groups. I don't know if it's worth attempting to guess alternative names for requirements.txt, though it may be beneficial to add support for Pipfile/pipenv.

I don't think pip-tools offers a huge lift, since upm already does enough of what pip-compile does today.

@cdmistman
Copy link
Contributor

I don't know if it's worth attempting to guess alternative names for requirements.txt, though it may be beneficial to add support for Pipfile/pipenv.

i think as long as we support other dependency groups with poetry it should be fine. i think the majority of repls use poetry

]
)
(import_from_statement
module_name: (dotted_name) @import)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
module_name: (dotted_name) @import)
module_name: (dotted_name) @import)

Copy link
Contributor

@cdmistman cdmistman left a comment

Choose a reason for hiding this comment

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

please reformat with standard lisp formatting - this formatting is usually easier for the author to read but harder for others to read 😅

Comment on lines +17 to +18
pkgs.python311Full
pkgs.python311Packages.pip
Copy link
Contributor

Choose a reason for hiding this comment

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

sanity check: the if cond: import thing syntax works with all 3.x right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since before 2.7 afaik

@blast-hardcheese
Copy link
Collaborator Author

please reformat with standard lisp formatting

Open to any suggestion as to how to make maintaining query formatting easier. Are there standard formatters you recommend for folk that don't know what "standard lisp formatting" looks like?

Copy link
Contributor

@cdmistman cdmistman left a comment

Choose a reason for hiding this comment

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

lgtm!

@blast-hardcheese blast-hardcheese merged commit d65eaaa into main Mar 8, 2024
3 checks passed
@blast-hardcheese blast-hardcheese deleted the dstewart/feat/python-guess-inside-ifstat branch March 8, 2024 21:56
@blast-hardcheese blast-hardcheese added enhancement New feature or request minor labels Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants