Some of the changes I suggested are outside the scope of this issue and should be left for another. Using DefaultDict is one issue. Removing print calls from tests is another. I think splitting the private function into two to simplify the test of the new feature can be included, but as a separate commit.

I made a separate commit for splitting _readmodule into two functions. It's only 5 lines, including whitespace and docstring. It didn't pass the tests at first and I've been debugging it for hours. It ended up that the calls back to _readmodule needed path and inpackage and I didn't include those at first. My question -- how could I have seen that sooner instead of spending hours debugging? It gave my a NameError once I figured out what to watch, but the new function didn't have those names defined, so I was surprised it failed silently.

Thanks!

I believe _Object should be private, if not removed. I asked on core-mentorship about the current added/changed notes as well as what is allowed in the doc if Object is removed.
EDIT: Object has a common method as well as data attributes, so I think to keep it as _Object.

In a previous version of this comment, I said
"readmodule() needs to be revised to only return toplevel classes, as was the case before this patch."
The need for revision is wrong as all nested Objects are in new .children attributes. Old code should never see the addition.

It is still true that there is no test of readmodule, and that one might be added in a new test revision issue. With a test, the implementation could be reduced to a dict comprehension.

Find in Files says neither _getname nor _getnamelist appear in test_pyclbr. Adding some would be another issue. Without knowing in detail what tokenizer emits, I am not sure they will handle legal multiline imports like
from tkinter import (Tk as #nasty split
k, mainloop as ml)

I assumed that any missing parameters would immediately result in NameErrors. If the test failed because of a missing parameter, and did not raise NameError, I would be surprised and puzzled. Looking at the test, one problem I see is that test_others() does multiple cm tests in sequence, so first to fail stops testing of the rest. If different files exercise different pathways with different needed parameters, it would take several runs to get them all. I am sorry that this was much more troublesome that I expected.

This function should be revised to use subtests (self.subTest) so all run regardless of failure. This is at least the fourth thing to fix in this file. Prints, temporary file, and missing tests are another.

After review, I think the only thing left for this issue is to make Object private. If I get no response on core mentorship, I will copy the current redundant doc format, but put Function first and note on Class that it has 2 more. The doc should also mention that Class.methods is redundant with Class.children in that the former lists the Functions in the latter. It is only kept for back-compatibility.

I am sorry that this was much more troublesome that I expected.

This wasn't a problem at all. Every chance I get to debug and track something down is more learning for me, so it's all good. I apologize if it sounded like I was complaining. I just brought it up because I was surprised I didn't get a warning while running the tests and to find if there were any tricks (or tools) that would have helped me reach the endpoint sooner.

Thanks for making all those changes. I know you said it needs more, but it looks awesome!

I think I did what I said was essential. Anyway, there were two conceptual improvements.

1. Expand the helper from 'create nested object' to 'add nested object'. This eliminated having to duplicate code when creating 'expected' in the test.

Added now: I believe the helpers should not just call .add_child, but should be incorporated into it, and the Class override.

2. Testing equality of the dicts failed because the default equality for Objects is by identity. Adding .eq failed because the parent-child-parent led to infinite recursion. Fiddling a bit with .eq did not work together. I then realized that I needed to compare nodes with data and children. So I paired None with its conceptual children to make a conceptual node. A top down equality test that tested each pair of nodes just once was then pretty easy to write.

Side node: general tree equality is hard because it is not initially known which child to pair with which. Here, the children for each node are uniquely labelled and the labels must match.

Added now. This is still clumsy. A possible solution: rename _Object as _Node. Replace None as the mod-level parent with top, an instance of _Node. Prime the _create_tree pump by initializing stack with [(top, -1)]. Since top is never popped, stack is never empty and we dump the stack test. Add all Function and Class node with (stack[-1][0]).add_child.

The creation of 'expected' in the test start with top = _Node(...) and all the addition would be with something.add_child. The compare function would have two parameters, node1 and node2, and would be .eq of _Node (where it could be tested).

_Node would then need one more method, .export_children. It would change the parent of all the children to None and then return the dict to be returned to users. Just where to do this would be a detail to be worked out after checking the tests.

I am not going to do this, unless revision is needed for IDLE. I would rather work on applying the lesson of matching code to concepts to IDLE.