Repository navigation
Add Python helpers - #195
Conversation
| # ------------------------------------------------------------------ | ||
|
|
||
| def test_extract_call_arguments_positional_only(self, mocker): | ||
| self._patch_factory(mocker, "fun(1, 'x')") |
There was a problem hiding this comment.
test documentation missing (ruff will complain)
There was a problem hiding this comment.
I can include more detailed descriptions of these tests, but I feel this would stand out as the majority of tests do not have docstrings attached to them. Do all tests contributed in the future require docstrings? And what should we do with old tests that do not have docstrings (yet)?
There was a problem hiding this comment.
In the archive all tests have docstrings - AI has added docstrings to all old tests that didn't have docstrings.
See 9f45459
As your pull request is made before this commit was pushed to main, this was not automatically reported.
Yet, merging in the main will lead to a failing CI/CD pipeline.
There was a problem hiding this comment.
See below: Ruff has rejected the change due to absence of documentation.
| assert_that(keyword, is_({"b": "'x'", "c": "other"})) | ||
|
|
||
| def test_extract_call_arguments_accepts_node_inside_call(self, mocker): | ||
| self._patch_factory(mocker, "fun(1, k='v')") |
There was a problem hiding this comment.
Does this only hold for the first node (child 0), all arguments, or even all nodes?
Why didn't you choose a hypothesis test?
What is the expected output for f(g(2,3))
- when node is
f(g(2,3))? - when node is
g(2,3)? - when node is 2?
There was a problem hiding this comment.
You raise a valid concern. I think it is maybe best to change the behaviour of this function by traversing from the passed node up the tree, until the first Call node is found. This way its behaviour is more well-defined and no doubts can arise from whether the passed node is actually a direct child from the Call node or not.
The expected outputs for your examples then would be:
positional_args:['g(2,3)'],keyword_args:{}positional_args:['2', '3'],keyword_args:{}positional_args:['2', '3'],keyword_args:{}
I haven't considered making a hypothesis test for this functionality. I think it would be a good addition to the existing suite, but I will first have to find out how they work.
| # ------------------------------------------------------------------ | ||
|
|
||
| def test_class_base_arguments_returns_declared_bases(self, mocker): | ||
| self._patch_factory(mocker, "class Child(Base1, Base2):\n pass") |
There was a problem hiding this comment.
What is the expected outcome for
class Middle(Base): pass
class Top(Middle): pass
when applied to Top?
Only Middle, Only Base, or [Middle, Base]?
There was a problem hiding this comment.
I will define the outcome of the function more rigorously. Unfortunately the function does not go up the inheritance chain. So it should rather read "what am I a direct child of?". For your example, it would return Middle.
There was a problem hiding this comment.
Could the name of the function be improved to better reflect this directness?
| class_node = subject.find_semantic_kind(SemanticKind.CLASS)[0] | ||
|
|
||
| assert_that(subject.class_inherits_from(class_node, "Base"), is_(True)) | ||
| assert_that(subject.class_inherits_from(class_node, "Other"), is_(False)) |
There was a problem hiding this comment.
Does every class inherit from object as object is defined in Python as "Any data with state (attributes or value) and defined behavior (methods). Also the ultimate base class of any new-style class."
There was a problem hiding this comment.
It is not configured to return object no. I will mention this in the docstring, and I will mention exactly what is being extracted (which comes down to what is in between the parentheses after the class declaration).
2deb75c to
df7aa5b
Compare
|
I have addressed all comments. Hopefully the PR is up to standard now. Let me know if you wish to see something different. |
pjljvandelaar
left a comment
pjljvandelaar
left a comment
There was a problem hiding this comment.
Great!
Nice addition. Thanks!
2 participants
Adds three helper methods that any Python recipe can access by operating on
self:extract_call_arguments: returns positional and keyword call arguments from a call node (or a child node within a call)class_base_arguments: returns declared base class signaturesclass_inherits_from: checks whether a class inherits from a given base nameTests for all methods have been provided as well, which clarifies intent of use cases as well.
Duplicate of #194, but this one has only verified commits.