| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
….partial and partialmethod objects
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
Corrected the NEWS entry
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
Updated to follow format
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
…ne_mark Partial and method objects cannot form reference cycles in this context, so the cycle-detection logic in _has_coroutine_mark introduced unnecessary overhead. This change removes the visited-set check and keeps the function as a simple unwrapping loop while still correctly detecting explicitly marked coroutine wrappers.
… cycle-detection logic
| # Functions created by partialmethod descriptors keep a __partialmethod__ reference | ||
| pm = getattr(f, "__partialmethod__", None) | ||
| if isinstance(pm, functools.partialmethod): | ||
| f = pm | ||
| continue |
There was a problem hiding this comment.
Well, this can also be moved forward by one block to avoid the time spent on obtaining the attribute when it is not necessary. I hope I have not bored you with these micro-optimizations.
Sorry, something went wrong.
| continue | ||
|
|
||
| # partial and partialmethod share .func | ||
| if isinstance(f, (functools.partial, functools.partialmethod)): |
There was a problem hiding this comment.
I will also add, "for the record", why I do not handle partialmethod objects in this way in the code attached to the original issue. They are not callable, and therefore applying markcoroutinefunction() to them is incorrect, which means they should not be checked. Being defined as a class member, accessing the corresponding attribute will return a regular function object created by partialmethod (or a method object for such a function, if via an instance). Therefore, there is no point in unnecessary iteration, and you can go straight to pm.func (see the block above).
Sorry, something went wrong.
|
|
||
| Interactive mode will start even when :data:`sys.stdin` does not appear to be a terminal. The | ||
| :envvar:`PYTHONSTARTUP` file is not read. | ||
| In these "execute then interact" cases, Python runs the script or command |
There was a problem hiding this comment.
This seems unrelated changes
Sorry, something went wrong.
| f = f.__func__ | ||
| f = functools._unwrap_partial(f) | ||
| return getattr(f, "_is_coroutine_marker", None) is _is_coroutine_mark | ||
| while True: |
There was a problem hiding this comment.
Please:
Sorry, something went wrong.
| return getattr(f, "_is_coroutine_marker", None) is _is_coroutine_mark | ||
| while True: | ||
| # Methods: unwrap first (methods cannot be coroutine-marked) | ||
| if ismethod(f): |
There was a problem hiding this comment.
Why not doing while ismethod(f): f = f.__func__ here?
Sorry, something went wrong.
There was a problem hiding this comment.
>>> from functools import partialmethod
>>> class MyClass:
... def a(self): ...
... b = partialmethod(a)
>>> obj = MyClass()
>>> obj.b
functools.partial(<bound method MyClass.a of <__main__.MyClass object at 0x7fd343f4cc20>>)
>>> obj.b.func.__func__
<function MyClass.a at 0x7fd343f42cf0>When unwrapping the reference to obj.b, the functools.partial object comes before the method object, so the assumption that method objects always precede other objects is incorrect.
>>> from functools import partialmethod
>>> class MyFirstClass:
... def f(self, other): ...
>>> first = MyFirstClass()
>>> class MySecondClass:
... g = partialmethod(first.f)
>>> second = MySecondClass()
>>> second.g
<bound method partialmethod._make_unbound_method.<locals>._method of <__main__.MySecondClass object at 0x7fd343f4d400>>
>>> second.g.__func__.__partialmethod__.func
<bound method MyFirstClass.f of <__main__.MyFirstClass object at 0x7fd343f4d2b0>>Well, it seems to me that the current implementation (in the main branch) is not very well thought out. Personally, I would not rely on it.
Sorry, something went wrong.
There was a problem hiding this comment.
If your comment is only about why there is if instead of while... why do we need a nested loop when we already have an outer one? Moreover, how often are methods that refer to other methods used? Does it even make sense to try to optimize this case? I think it is sufficient and simpler to rely on the outer loop.
As you can easily see, the code in this PR is very similar to the one I attached to the original issue, except for the order of the blocks and some points borrowed from the original code (AI-generated or just copy-paste?). I use something similar in the implementation of similar functions in my library, although they are more general in nature.
Sorry, something went wrong.
| # Functions created by partialmethod descriptors keep a __partialmethod__ reference | ||
| pm = getattr(f, "__partialmethod__", None) | ||
| if isinstance(pm, functools.partialmethod): | ||
| f = pm | ||
| continue |
There was a problem hiding this comment.
I think you should use functools._unwrap_partialmethod which handles both partial methods and partial functions (first it unwraps partial methods then unwraps partial functions)
Sorry, something went wrong.
There was a problem hiding this comment.
Please read the discussion at #142505.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes gh-142418.
This PR updates inspect.iscoroutinefunction() so that coroutine markers
applied via inspect.markcoroutinefunction() are correctly detected on
wrapped callables, including functools.partial and
functools.partialmethod objects.
Previously, iscoroutinefunction() only checked the marker after unwrapping
the callable, which caused false negatives for marked partial and
partialmethod objects. The new logic checks for the marker at each unwrap
stage, with cycle protection, ensuring that any explicitly marked wrapper is
recognized as a coroutine function.
This change also adds regression tests verifying correct behavior for marked
and unmarked functools.partial and functools.partialmethod objects, and
includes a NEWS entry documenting the fix.
📚 Documentation preview 📚: https://cpython-previews--142503.org.readthedocs.build/