bpo-8525: help() on a type now shows builtin subclasses #5066

CuriousLearner · 2017-12-31T18:49:21Z

https://bugs.python.org/issue8525

CuriousLearner · 2018-01-06T09:25:44Z

So, seems like it when this random seed 7657408 is used to run the tests, when they fail the test test_unget_wch.

I'm not sure how do we particularly debug and fix the issue.

@vstinner Can you please help?

CuriousLearner · 2018-01-06T12:13:10Z

The test passes on running a separate build for the same change on my own fork: https://travis-ci.org/CuriousLearner/cpython/builds/325741101

vstinner · 2018-01-20T00:40:59Z

@CuriousLearner: Sorry, I don't understand your comment. What do you expect from me? Your PR changes pydoc, but I tested test_pydoc locally: it pass. So what is your problem?

CuriousLearner · 2018-01-20T08:36:21Z

@vstinner So, the issue is that the tests passes. But they fail when they run in a particular order; probably right now when randseed is 7657408. Can you guide me how can I debug this behavior?
I ran another travis build on my own fork and it started with another randseed which seems to pass: https://travis-ci.org/CuriousLearner/cpython/builds/325741101

How can I debug this particular behaviour ?

vstinner · 2018-01-20T09:12:22Z

On failure, which test fails? With which error message? Copy the failure here.

CuriousLearner · 2018-01-20T16:11:33Z

@vstinner Sorry, if it was not clear. It is the Travis build in this PR that failed.

Here is the failed test:

======================================================================
FAIL: test_semaphore_tracker (test.test_multiprocessing_fork.TestSemaphoreTracker)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/python/cpython/Lib/test/_test_multiprocessing.py", line 4380, in test_semaphore_tracker
    _multiprocessing.sem_unlink(name2)
AssertionError: OSError not raised

And here are the logs: https://travis-ci.org/python/cpython/jobs/323647995#L2592

vstinner · 2018-01-20T21:19:48Z

FAIL: test_semaphore_tracker (test.test_multiprocessing_fork.TestSemaphoreTracker)

Oh, this one... It's unrelated to your change. It's a race condition in test_multiprocessing_fork: I guess that the test fails depending on the system load. Sadly, it's something common in the Python test suite, even if we are trying to fix all these race conditions one by one, when one bug becomes too often on our CI :-)

See https://bugs.python.org/issue31687

To reproduce the issue, you should increase the system load to make your system slower and make timeouts more likely, and change the order in which processes are run. I'm using the "stress" utility on Linux for that. But sometimes, the best stress tool... is the Python test suite, since running the full test suite in multiple processes in parallel (./python -m test -j0 -r) runs various workloads, and so sometimes trigger bugs which would be very hard to trigger othewise.

Terminal 1: run "./python -m test -j0 -r", stress or anything else to make the system slower

Terminal 2: run "./python -m test test_multiprocessing_fork -m test_semaphore_tracker -F -v" which runs the test in a loop until it fails.

eric-wieser · 2018-02-20T00:57:44Z

Lib/pydoc.py

+        # List the built-in subclasses, if any:
+        subclasses = [
+            cls.__name__ for cls in object.__subclasses__()
+            if cls.__module__ == 'builtins'


Elsewhere in this file we test against object.__module__ - I think this is relevant for python 2, but consistency seems useful anyway.

Ah nevermind - this is not the builtin object.

eric-wieser · 2018-02-20T01:00:54Z

What does help(object) now print?

CuriousLearner · 2018-02-20T17:56:46Z

@eric-wieser The help(object) now returns the following:

Help on class object in module builtins:

class object
 |  The most base type
 |
 |  Built-in subclasses:
 |      BaseException
 |      EncodingMap
 |      NoneType
 |      NotImplementedType
 |      PyCapsule
 |      async_generator
 |      builtin_function_or_method
 |      bytearray
 |      bytearray_iterator
 |      bytes
 |      bytes_iterator
 |      callable_iterator
 |      cell
 |      classmethod
 |      classmethod_descriptor
 |      code
 |      complex
 |      coroutine
 |      coroutine_wrapper
 |      dict
 |      dict_itemiterator
 |      dict_items
 |      dict_keyiterator
 |      dict_keys
 |      dict_valueiterator
 |      dict_values
 |      ellipsis
 |      enumerate
 |      fieldnameiterator
 |      filter
 |      float
 |      formatteriterator
 |      frame
 |      frozenset
 |      function
 |      generator
 |      getset_descriptor
 |      instancemethod
 |      int
 |      iterator
 |      list
 |      list_iterator
 |      list_reverseiterator
 |      longrange_iterator
 |      managedbuffer
 |      map
 |      mappingproxy
 |      member_descriptor
 |      memoryview
 |      method
 |      method-wrapper
 |      method_descriptor
 |      module
 |      moduledef
 |      odict_iterator
 |      property
 |      range
 |      range_iterator
 |      reversed
 |      set
 |      set_iterator
 |      slice
 |      staticmethod
 |      stderrprinter
 |      str
 |      str_iterator
 |      super
 |      traceback
 |      tuple
 |      tuple_iterator
 |      type
 |      weakcallableproxy
 |      weakproxy
 |      weakref
 |      wrapper_descriptor
 |      zip

eric-wieser · 2018-02-20T18:22:45Z

That strikes me as thoroughly unhelpful - perhaps it should not print anything for object?

CuriousLearner · 2018-02-20T18:44:30Z

@eric-wieser Sure. Sounds good to me. I've made some modifications so that we don't push the sub-classes details for object

eric-wieser · 2018-02-20T18:48:41Z

Lib/pydoc.py

+            cls.__name__ for cls in object.__subclasses__()
+            if cls.__module__ == 'builtins'
+        ]
+        if subclasses and object.__class__ != type:


Are you sure you didn't want object is not builtins.object? What you have now only shows subclasses for objects with a metaclass.

Oh, yes. You're correct! I've updated the patch to reflect the changes.

CuriousLearner · 2018-07-14T08:16:24Z

@ncoghlan @vstinner Can you please take a look here?

ncoghlan · 2018-07-14T12:47:19Z

See my comments at https://bugs.python.org/issue8525#msg321651

If we do make this change, it's going to need a hard upper limit on the number of subclasses shown, even within the exception hierarchy.

* master: (1159 commits) bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274) bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271) bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924) bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267) bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261) bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222) Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887) bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192) bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114) bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245) Simplify __all__ in multiprocessing (pythonGH-6856) bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230) Doc: Point to Simple statements section instead of PEP (pythonGH-8238) bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139) bpo-33597: Add What's New for PyGC_Head (pythonGH-8236) Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038) Fix documentation for input and output tutorial (pythonGH-8231) bpo-34009: Expand on platform support changes (pythonGH-8022) Factor-out two substantially identical code blocks. (pythonGH-8219) bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091) ...

CuriousLearner · 2018-07-15T09:38:44Z

@ncoghlan I've made the changes to display at most 4 subclasses.

But IMHO, there should be also a way to see all the subclasses if I want to see. That could be a pretty useful feature for folks. What do you think?

CuriousLearner · 2018-07-21T09:55:16Z

@ncoghlan I have made the requested changes; please review again

bedevere-bot · 2018-07-21T09:55:18Z

Thanks for making the requested changes!

: please review the changes made to this pull request.

CuriousLearner · 2018-07-22T06:38:13Z

@ncoghlan I'm not sure how the no. of subclasses changes on my local and the CI. That is why the tests seem to fail on CI.

I'm curious to know on what's the reasoning behind this.

eric-wieser · 2018-07-23T15:46:26Z

Lib/pydoc.py

@@ -1254,6 +1254,24 @@ def makename(c, m=object.__module__):
                push('    ' + makename(base))
            push('')

+        # List the built-in subclasses, if any:
+        subclasses = sorted(
+            (str(cls.__name__) for cls in object.__subclasses__() \


Backslash not needed here

Oh, nice catch! Fixed this :)

Thank you!

CuriousLearner · 2018-10-07T13:45:43Z

Hey @ncoghlan @vstinner

I think this patch was really close (and it has been 3 months since the last update :) ), I would really appreciate your help if you can guide me about how the number of subclasses is varying on locally and on CI so that I can fix these up :)

Are these OS dependent?

* master: (621 commits) Update opcode.h header comment to mention the source data file (pythonGH-9935) bpo-34936: Fix TclError in tkinter.Spinbox.selection_element(). (pythonGH-9760) Updated documentation on logging.debug(). (pythonGH-9946) bpo-34765: Update the install-sh file (pythonGH-9592) bpo-35008: Fix possible leaks in Element.__setstate__(). (pythonGH-9924) bpo-35011: Restore use of pyexpatns.h in libexpat (pythonGH-9939) bpo-24658: Fix read/write greater than 2 GiB on macOS (pythonGH-1705) Add missing comma to wsgiref doc (pythonGH-9932) bpo-23420: Verify the value of '-s' when execute the CLI of cProfile (pythonGH-9925) Doc: Fix is_prime (pythonGH-9909) In email docs, correct spelling of foregoing (python#9856) In email.parser in message_from_bytes, update `strict` to `policy` (python#9854) bpo-34997: Fix test_logging.ConfigDictTest.test_out_of_order (pythonGH-9913) Added CLI starter example to logging cookbook. (pythonGH-9910) bpo-34783: Fix test_nonexisting_script() (pythonGH-9896) bpo-23554: Change echo server example class name from EchoServerClientProtocol to EchoServerProtocol (pythonGH-9859) bpo-34989: python-gdb.py: fix current_line_num() (pythonGH-9889) Stop using deprecated logging API in Sphinx suspicious checker (pythonGH-9875) fix dangling keyfunc examples in documentation of heapq and sorted (python#1432) bpo-34844: logging.Formatter enhancement - Ensure style and format string matches in logging.Formatter (pythonGH-9703) ...

CuriousLearner · 2018-10-18T22:39:19Z

Hi, @ncoghlan @vstinner I've fixed up everything. CI is all good now.

Can you please take a look?

willingc

Thanks for persisting @CuriousLearner.

ncoghlan

Functionally, I think this version is OK (and counts as an improvement over the status quo, even if further changes end up being made).

Hardcoding an exact number of subclasses into the test case is going to be annoying to maintain though (e.g. it already needs to be skipped Windows), so I think relaxing that check to just require it to be a number (\d+ in a regex) is fine.

ncoghlan · 2018-10-21T04:19:48Z

Lib/test/test_pydoc.py

+                " |      BaseException\n"
+                " |      builtin_function_or_method\n"
+                " |      bytearray\n"
+                " |      ... and 86 other subclasses")


Hardcoding the number here is going to make the test overly fragile, so I'd suggest making this a regex and using assertRegexMatches instead of assertIn.

ncoghlan · 2018-10-21T04:21:51Z

Lib/test/test_pydoc.py

@@ -517,6 +517,38 @@ def test_stripid(self):
        self.assertEqual(stripid("<type 'exceptions.Exception'>"),
                         "<type 'exceptions.Exception'>")

+    @unittest.skipIf(sys.platform == 'win32', "Different no of subclasses on win")


With the regex change below, it won't be necessary to skip this test on Windows any more.

bedevere-bot · 2018-10-21T06:44:10Z

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.

ncoghlan

I went ahead and made the assertIn -> assertRegex change, so assuming that passes CI, I think this will be good to go.

ncoghlan · 2018-10-21T07:20:08Z

Also tweaked NEWS entry wording and PR title after GH prompted me to edit the commit message on the squash commit.

bpo-8525: help() now shows exceptions' builtin subclasses

ee6dfa8

the-knights-who-say-ni added the CLA signed label Dec 31, 2017

bedevere-bot added the awaiting review label Dec 31, 2017

eric-wieser reviewed Feb 20, 2018

View reviewed changes

bpo-8525: Don't display subclasses for object

bd313ff

eric-wieser reviewed Feb 20, 2018

View reviewed changes

bpo-8525: Address review comments

defa5a9

eric-wieser approved these changes Feb 22, 2018

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Feb 22, 2018

CuriousLearner added 2 commits July 14, 2018 20:03

Show maximum of 4 subclasses

3d551f0

bedevere-bot added awaiting change review and removed awaiting core review labels Jul 21, 2018

eric-wieser reviewed Jul 23, 2018

View reviewed changes

Address review comments

9199480

willingc self-requested a review October 7, 2018 09:24

CuriousLearner added 2 commits October 19, 2018 03:16

Skip test_builtin_with_more_than_four_children for windows

911c326

willingc approved these changes Oct 18, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Oct 18, 2018

ncoghlan requested changes Oct 21, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting merge labels Oct 21, 2018

Use assertRegex to handle variable output

8d30a30

ncoghlan approved these changes Oct 21, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Oct 21, 2018

ncoghlan changed the title ~~bpo-8525: help() now shows exceptions' builtin subclasses~~ bpo-8525: help() on a type now shows builtin subclasses Oct 21, 2018

Reword NEWS entry based on changes to the patch

677f053

ncoghlan merged commit a323cdc into python:master Oct 21, 2018

bedevere-bot removed the awaiting merge label Oct 21, 2018

Carreau mentioned this pull request Oct 22, 2018

Show subclasses when using pinfo/pinfo2/?/?? ipython/ipython#11432

Open

Uh oh!

bpo-8525: help() on a type now shows builtin subclasses #5066

bpo-8525: help() on a type now shows builtin subclasses #5066

Uh oh!

Conversation

CuriousLearner commented Dec 31, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CuriousLearner commented Jan 6, 2018

Uh oh!

CuriousLearner commented Jan 6, 2018

Uh oh!

vstinner commented Jan 20, 2018

Uh oh!

CuriousLearner commented Jan 20, 2018

Uh oh!

vstinner commented Jan 20, 2018 via email

Uh oh!

CuriousLearner commented Jan 20, 2018

Uh oh!

vstinner commented Jan 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented Feb 20, 2018

Uh oh!

CuriousLearner commented Feb 20, 2018

Uh oh!

eric-wieser commented Feb 20, 2018

Uh oh!

CuriousLearner commented Feb 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CuriousLearner commented Jul 14, 2018

Uh oh!

ncoghlan commented Jul 14, 2018

Uh oh!

CuriousLearner commented Jul 15, 2018

Uh oh!

CuriousLearner commented Jul 21, 2018

Uh oh!

bedevere-bot commented Jul 21, 2018

Uh oh!

CuriousLearner commented Jul 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CuriousLearner commented Oct 7, 2018

Uh oh!

CuriousLearner commented Oct 18, 2018

Uh oh!

willingc left a comment

Choose a reason for hiding this comment

Uh oh!

ncoghlan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 21, 2018

Uh oh!

ncoghlan left a comment

Choose a reason for hiding this comment

Uh oh!

ncoghlan commented Oct 21, 2018

Uh oh!

Uh oh!

CuriousLearner commented Dec 31, 2017 •

edited by bedevere-bot

Loading

ncoghlan left a comment •

edited

Loading