Opened 3 years ago

Last modified 16 months ago

#5644 enhancement new

t.p.reflect.fullyQualifiedName doesn't support method descriptors.

Reported by: dreid Owned by: dreid
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/fullyQualifiedName-methoddescriptor-5644-3
(diff, github, buildbot, log)
Author: dreid Launchpad Bug:

Description

>>> fullyQualifiedName(set.add)
'add'

when the correct FQN is __builtin__.set.add

namedAny already does the right thing when handed __builtin__.set.add

>>> namedAny('__builtin__.set.add')
<method 'add' of 'set' objects>

Change History (14)

comment:1 Changed 3 years ago by dreid

  • Author set to dreid
  • Branch set to branches/fullyQualifiedName-methoddescriptor-5644

(In [34196]) Branching to 'fullyQualifiedName-methoddescriptor-5644'

comment:2 Changed 3 years ago by dreid

  • Keywords review added

comment:4 Changed 3 years ago by thijs

  • Keywords review removed
  • Owner set to dreid

These are doc issues in t.python.deprecate not directly related to your code but simple enough to fix while we're in that module:

  • in getDeprecationWarningString the docstring has duplicate @param definitions
  • same for @param version in deprecated
  • warnAboutFunction documents a function param but this should be offender

Also needs a feature or bugfix news file.

comment:5 Changed 3 years ago by dreid

  • Keywords review added
  • Owner dreid deleted

I addressed the unrelated docstring issues and added a news fragment.

comment:6 Changed 2 years ago by exarkun

  • Keywords review removed
  • Owner set to dreid

Thanks. Two points:

  1. in warnAboutFunction, C{function} doesn't make much sense. Just function, or perhaps L{types.FunctionType}, or maybe no-argument callable would be better.
  2. inspect.ismethoddescriptor looks like a sort of useless predict. The documentation says all it guarantees is the argument has __get__ and does not have __set__ and that some of the other inspect.is* functions would return False. In particular, it says nothing at all about __objclass__. Should we just check for the existence of __objclass__? These kinds of objects seem pretty gross and obscure. :/

comment:7 Changed 2 years ago by dreid

  • Branch changed from branches/fullyQualifiedName-methoddescriptor-5644 to branches/fullyQualifiedName-methoddescriptor-5644-2

(In [34761]) Branching to 'fullyQualifiedName-methoddescriptor-5644-2'

comment:8 Changed 2 years ago by dreid

(In [34762]) Merge forward. refs #5644

comment:9 Changed 17 months ago by dreid

  • Branch changed from branches/fullyQualifiedName-methoddescriptor-5644-2 to branches/fullyQualifiedName-methoddescriptor-5644-3

(In [38662]) Branching to fullyQualifiedName-methoddescriptor-5644-3.

comment:10 Changed 17 months ago by dreid

(In [38663]) Merging forward. Refs #5644

comment:11 Changed 17 months ago by dreid

(In [38664]) Parameterize expected value because int.module differs between py2 and py3. Refs #5644

comment:12 Changed 17 months ago by dreid

  • Keywords review added
  • Owner dreid deleted

comment:13 Changed 16 months ago by exarkun

  • Keywords review removed
  • Owner set to dreid

Thanks! I love fullyQualifiedName and I love fixes to its handling of dark corner cases even more.

  1. in twisted/python/deprecate.py
    1. Please use getattr instead of hasattr - particularly when the object is arbitrary and application-supplied.
  2. There are some confusing deletions in the branch that seem... accidental? Docstrings of getDeprecationWarningString and deprecated and test_boundMethod seem worse than they were before.

Otherwise seems good, but I'm somewhat confused by (2) so I don't want to say "fix those and merge".

comment:14 Changed 16 months ago by exarkun

Oh yes, one more thing I forgot, http://buildbot.twistedmatrix.com/builders/python-3.3-tests/builds/932/steps/shell/logs/stdio has a failure:

======================================================================
FAIL: test_circularChainWarning (twisted.test.test_defer.DeferredTestCase)
test_circularChainWarning
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/var/lib/buildbot/buildarea/python-3.3-tests/Twisted/twisted/trial/_synctest.py", line 1211, in _run
    runWithWarningsSuppressed(suppress, method)
  File "/var/lib/buildbot/buildarea/python-3.3-tests/Twisted/twisted/python/util.py", line 1065, in runWithWarningsSuppressed
    return f(*args, **kwargs)
  File "/var/lib/buildbot/buildarea/python-3.3-tests/Twisted/twisted/test/test_defer.py", line 922, in test_circularChainWarning
    "\nExpected match: %r\nGot: %r" % (pattern, warning['message']))
  File "/var/lib/buildbot/buildarea/python-3.3-tests/Twisted/twisted/trial/_synctest.py", line 308, in assertTrue
    raise self.failureException(msg)
twisted.trial.unittest.FailTest: 
Expected match: 'Callback <function circularCallback at 0x\\w+> returned the same Deferred it was attached to'
Got: 'Callback <function DeferredTestCase.test_circularChainWarning.<locals>.circularCallback at 0x7fced8c6ecb0> returned the same Deferred it was attached to; this breaks the callback chain and will raise an exception in the future.'

----------------------------------------------------------------------
Note: See TracTickets for help on using tickets.