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

[refactoring] Extract helper method has_specific_arg #49

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

izaitsevfb
Copy link
Contributor

@izaitsevfb izaitsevfb commented Apr 22, 2024

fix #8,
Extracted helper method has_specific_arg that checks for the call argument presence, and simplified all relevant call sites:

        qualified_name = self.get_qualified_name_for_call(node)
        if qualified_name == "torch.load":
            weights_only_arg = self.get_specific_arg(node, "weights_only", -1)
            if weights_only_arg is None:

becomes:

       if self.get_qualified_name_for_call(node) == "torch.load" and \
                not self.has_specific_arg(node, "weights_only"):

Testing

pytest tests

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 22, 2024
@izaitsevfb izaitsevfb force-pushed the refactoring-extract-has-specific-arg branch from 6b51aae to 4f4cec5 Compare April 22, 2024 20:04
@izaitsevfb izaitsevfb requested a review from kit1980 April 22, 2024 20:05
@kit1980
Copy link
Contributor

kit1980 commented Apr 22, 2024

get_specific_arg is already there.
You mean has_specific_arg, right? Please update title/description.

@izaitsevfb izaitsevfb changed the title [refactoring] Extract helper method get_specific_arg [refactoring] Extract helper method has_specific_arg Apr 22, 2024
@izaitsevfb izaitsevfb changed the title [refactoring] Extract helper method has_specific_arg [refactoring] Extract helper method has_specific_arg Apr 22, 2024
@izaitsevfb
Copy link
Contributor Author

get_specific_arg is already there. You mean has_specific_arg, right? Please update title/description.

good catch, thanks. Let me reword the commit as well.

…call argument presence, and simplify all relevant call sites
@izaitsevfb izaitsevfb force-pushed the refactoring-extract-has-specific-arg branch from 4f4cec5 to 2635be0 Compare April 22, 2024 20:07
node: cst.Call, arg_name: str, position: Optional[int] = None
) -> bool:
"""
Check if a named argument is present in a call (not positional).
Copy link
Contributor

@kit1980 kit1980 Apr 22, 2024

Choose a reason for hiding this comment

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

not positional

What if position passed?
I guess either delete position parameter or change the comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@izaitsevfb izaitsevfb merged commit af37f69 into main Apr 22, 2024
2 checks passed
@izaitsevfb izaitsevfb deleted the refactoring-extract-has-specific-arg branch April 22, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Try to refactor common functionality for checking default args
4 participants