From c32f39b52a1c81a8ca91652480496f2b8e6e23b4 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 6 Dec 2024 15:56:57 -0500 Subject: [PATCH 01/20] Fix #846 --- README.rst | 3 ++ pydoctor/model.py | 66 +++++++++++++++++++++++++++++++-------- pydoctor/test/test_mro.py | 44 ++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/README.rst b/README.rst index c57e56bf4..a004473db 100644 --- a/README.rst +++ b/README.rst @@ -73,6 +73,9 @@ What's New? in development ^^^^^^^^^^^^^^ +* Fix a bug in the MRO computing code that would result in an incorrect + ``Cannot compute linearization of the class inheritance hierarchy`` message + for valid types extending ``typing.Generic`` as well as other generic classes. * Fix a bug that would cause a variable marked as `Final` not being considered as a constant if it was declared under a control-flow block. * Fix a bug in google and numpy "Attributes" section in module docstring: diff --git a/pydoctor/model.py b/pydoctor/model.py index 9c78fc3d0..1439a7674 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -9,6 +9,7 @@ import abc import ast +from functools import partial from itertools import chain from collections import defaultdict import datetime @@ -582,7 +583,7 @@ def is_exception(cls: 'Class') -> bool: return True return False -def compute_mro(cls:'Class') -> Sequence[Union['Class', str]]: +def compute_mro(cls:'Class', cleanup_generics: bool = False) -> Sequence[Class | str]: """ Compute the method resolution order for this class. This function will also set the @@ -622,23 +623,59 @@ def init_finalbaseobjects(o: 'Class', path:Optional[List['Class']]=None) -> None o._finalbaseobjects = finalbaseobjects o._finalbases = finalbases - def localbases(o:'Class') -> Iterator[Union['Class', str]]: + # Since the typing.Generic can be listed more than once in the class hierarchy: + # ignore the ones after the first discovered one. + _bases: dict[Class | str, list[Class | str]] = {} + _has_generic: bool = False + def _getbases(o:'Class', ignore_generic: bool = False) -> Iterator[Class | str]: """ - Like L{Class.baseobjects} but fallback to the expanded name if the base is not resolved to a L{Class} object. + Like L{Class.baseobjects} but fallback to the expanded + name if the base is not resolved to a L{Class} object. + + As well as handle multiple typing.Generic case, + see https://github.com/twisted/pydoctor/issues/846. """ for s,b in zip(o.bases, o.baseobjects): if isinstance(b, Class): yield b else: - yield s + # yield s + # Should we make it work event when typing.py is part of the system ? + # since pydoctor is not used to document the standard library + # it's probably not worth it... + if s == 'typing.Generic': + nonlocal _has_generic + _has_generic = True + if not ignore_generic: + yield s + else: + continue + else: + yield s + + def getbases(o:Class | str) -> list[Class | str]: + nonlocal _getbases - def getbases(o:Union['Class', str]) -> List[Union['Class', str]]: if isinstance(o, str): return [] - return list(localbases(o)) + + if o in _bases: + return _bases[o] + + r = list(_getbases(o)) + _bases[o] = r + + if _has_generic and cleanup_generics: + _getbases = partial(_getbases, ignore_generic=True) + + return r init_finalbaseobjects(cls) - return mro.mro(cls, getbases) + _mro = mro.mro(cls, getbases) + + if cleanup_generics: + _mro.sort(key=lambda c: c == 'typing.Generic') + return _mro def _find_dunder_constructor(cls:'Class') -> Optional['Function']: """ @@ -698,7 +735,7 @@ class Class(CanContainImportsDocumentable): # set in post-processing: _finalbaseobjects: Optional[List[Optional['Class']]] = None _finalbases: Optional[List[str]] = None - _mro: Optional[Sequence[Union['Class', str]]] = None + _mro: Optional[Sequence[Class | str]] = None def setup(self) -> None: super().setup() @@ -714,15 +751,18 @@ def _init_mro(self) -> None: """ try: self._mro = compute_mro(self) - except ValueError as e: - self.report(str(e), 'mro') - self._mro = list(self.allbases(True)) + except ValueError: + try: + self._mro = compute_mro(self, cleanup_generics=True) + except ValueError as e: + self.report(str(e), 'mro') + self._mro = list(self.allbases(True)) @overload - def mro(self, include_external:'Literal[True]', include_self:bool=True) -> Sequence[Union['Class', str]]:... + def mro(self, include_external:'Literal[True]', include_self:bool=True) -> Sequence[Class | str]:... @overload def mro(self, include_external:'Literal[False]'=False, include_self:bool=True) -> Sequence['Class']:... - def mro(self, include_external:bool=False, include_self:bool=True) -> Sequence[Union['Class', str]]: + def mro(self, include_external:bool=False, include_self:bool=True) -> Sequence[Class | str]: """ Get the method resution order of this class. diff --git a/pydoctor/test/test_mro.py b/pydoctor/test/test_mro.py index 2c96dc1a3..53acb809d 100644 --- a/pydoctor/test/test_mro.py +++ b/pydoctor/test/test_mro.py @@ -130,6 +130,50 @@ class D(C):... cycle:4: Cycle found while computing inheritance hierarchy: cycle.D -> cycle.C -> cycle.A -> cycle.D ''' +@systemcls_param +def test_mro_generic(systemcls: Type[model.System], capsys:CapSys) -> None: + src = ''' + from typing import Generic, TypeVar + T = TypeVar('T') + class A: ... + class B(Generic[T]): ... + class C(A, Generic[T], B[T]): ... + ''' + mod = fromText(src, modname='t', systemcls=systemcls) + assert not capsys.readouterr().out + assert_mro_equals(mod.contents['C'], + ["t.C", "t.A", "t.B", "typing.Generic"]) + +@systemcls_param +def test_mro_generic_2(systemcls: Type[model.System], capsys:CapSys) -> None: + src = ''' + from typing import Generic, TypeVar + T1 = TypeVar('T1') + T2 = TypeVar('T2') + class A(Generic[T1]): ... + class B(Generic[T2]): ... + class C(A[T1], B[T2]): ... + ''' + mod = fromText(src, modname='t', systemcls=systemcls) + assert not capsys.readouterr().out + assert_mro_equals(mod.contents['C'], + ["t.C", "t.A", "t.B", "typing.Generic"]) + +@systemcls_param +def test_mro_generic_3(systemcls: Type[model.System], capsys:CapSys) -> None: + src = ''' + from typing import Generic as TGeneric, TypeVar + T = TypeVar('T') + class Generic: ... + class A(Generic): ... + class B(TGeneric[T]): ... + class C(A, B[T]): ... + ''' + mod = fromText(src, modname='t', systemcls=systemcls) + assert not capsys.readouterr().out + assert_mro_equals(mod.contents['C'], + ["t.C", "t.A", "t.Generic", "t.B", "typing.Generic"]) + def test_inherited_docsources()-> None: simple = fromText("""\ class A: From 2563cd2709062b462b9777f02d5d53e856a74821 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 6 Dec 2024 15:58:25 -0500 Subject: [PATCH 02/20] Remove commented code --- pydoctor/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 1439a7674..12e949659 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -639,7 +639,6 @@ def _getbases(o:'Class', ignore_generic: bool = False) -> Iterator[Class | str]: if isinstance(b, Class): yield b else: - # yield s # Should we make it work event when typing.py is part of the system ? # since pydoctor is not used to document the standard library # it's probably not worth it... From 2cd1e9dcf6dd56fb935d0cd3a767c0ebfa148bc1 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Fri, 6 Dec 2024 16:02:41 -0500 Subject: [PATCH 03/20] Add some typing --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 12e949659..89379a6d7 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -670,7 +670,7 @@ def getbases(o:Class | str) -> list[Class | str]: return r init_finalbaseobjects(cls) - _mro = mro.mro(cls, getbases) + _mro: list[Class | str] = mro.mro(cls, getbases) if cleanup_generics: _mro.sort(key=lambda c: c == 'typing.Generic') From 6993254a372a2dd929105fb8432f1bb0b4714005 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Fri, 6 Dec 2024 16:04:47 -0500 Subject: [PATCH 04/20] Add comment --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 89379a6d7..d30e2ed92 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -624,7 +624,7 @@ def init_finalbaseobjects(o: 'Class', path:Optional[List['Class']]=None) -> None o._finalbases = finalbases # Since the typing.Generic can be listed more than once in the class hierarchy: - # ignore the ones after the first discovered one. + # ignore the ones after the first discovered one. Then sort the MRO to put typing.Generic at the end. _bases: dict[Class | str, list[Class | str]] = {} _has_generic: bool = False def _getbases(o:'Class', ignore_generic: bool = False) -> Iterator[Class | str]: From 78522b2d33fa12fc0d528a66bf768104575cd68a Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 17:55:50 -0500 Subject: [PATCH 05/20] Fix implementation of MRO post-processing, do it the right way. It should be adjustable for future tweaks like the one for typing.Generic. --- pydoctor/model.py | 153 +++++++++++++--------- pydoctor/mro.py | 13 +- pydoctor/test/test_mro.py | 42 +++--- pydoctor/topsort.py | 268 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 384 insertions(+), 92 deletions(-) create mode 100644 pydoctor/topsort.py diff --git a/pydoctor/model.py b/pydoctor/model.py index 89379a6d7..b89b78e4d 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -22,7 +22,7 @@ from inspect import signature, Signature from pathlib import Path from typing import ( - TYPE_CHECKING, Any, Collection, Dict, Iterator, List, Mapping, Callable, + TYPE_CHECKING, Any, Collection, Dict, Iterable, Iterator, List, Mapping, Callable, Optional, Sequence, Set, Tuple, Type, TypeVar, Union, cast, overload ) from urllib.parse import quote @@ -33,9 +33,10 @@ from pydoctor import factory, qnmatch, utils, linker, astutils, mro from pydoctor.epydoc.markup import ParsedDocstring from pydoctor.sphinx import CacheT, SphinxInventory +from pydoctor.topsort import topsort if TYPE_CHECKING: - from typing_extensions import Literal, Protocol + from typing import Literal, Protocol, TypeAlias from pydoctor.astbuilder import ASTBuilder, DocumentableT else: Literal = {True: bool, False: bool} @@ -583,21 +584,27 @@ def is_exception(cls: 'Class') -> bool: return True return False -def compute_mro(cls:'Class', cleanup_generics: bool = False) -> Sequence[Class | str]: +_ClassOrStr: TypeAlias = 'Class | str' + +class ClassHierarchyFinalizer: """ - Compute the method resolution order for this class. - This function will also set the - C{_finalbaseobjects} and C{_finalbases} attributes on - this class and all it's superclasses. + Encapsulate code related to class hierarchies post-processing. """ - def init_finalbaseobjects(o: 'Class', path:Optional[List['Class']]=None) -> None: + + @staticmethod + def _init_finalbaseobjects(o: Class, path:list[Class] | None = None) -> None: + """ + The base objects are computed in two passes, first the ast visitor sets C{_initialbaseobjects}, + then we set C{_finalbaseobjects} from this function which should be called during post-processing. + """ if not path: path = [] if o in path: - cycle_str = " -> ".join([o.fullName() for o in path[path.index(cls):] + [cls]]) + cycle_str = " -> ".join([c.fullName() for c in path[path.index(o):] + [o]]) raise ValueError(f"Cycle found while computing inheritance hierarchy: {cycle_str}") path.append(o) if o._finalbaseobjects is not None: + # we already computed these, so skip. return if o.rawbases: finalbaseobjects: List[Optional[Class]] = [] @@ -619,62 +626,88 @@ def init_finalbaseobjects(o: 'Class', path:Optional[List['Class']]=None) -> None finalbases.append(o._initialbases[i]) if base: # Recurse on super classes - init_finalbaseobjects(base, path.copy()) + ClassHierarchyFinalizer._init_finalbaseobjects(base, path.copy()) o._finalbaseobjects = finalbaseobjects o._finalbases = finalbases - - # Since the typing.Generic can be listed more than once in the class hierarchy: - # ignore the ones after the first discovered one. - _bases: dict[Class | str, list[Class | str]] = {} - _has_generic: bool = False - def _getbases(o:'Class', ignore_generic: bool = False) -> Iterator[Class | str]: + + @staticmethod + def _getbases(o: Class) -> Iterator[_ClassOrStr]: """ Like L{Class.baseobjects} but fallback to the expanded name if the base is not resolved to a L{Class} object. - - As well as handle multiple typing.Generic case, - see https://github.com/twisted/pydoctor/issues/846. """ for s,b in zip(o.bases, o.baseobjects): if isinstance(b, Class): yield b else: - # Should we make it work event when typing.py is part of the system ? - # since pydoctor is not used to document the standard library - # it's probably not worth it... - if s == 'typing.Generic': - nonlocal _has_generic - _has_generic = True - if not ignore_generic: - yield s - else: - continue - else: - yield s + yield s + + def __init__(self, classes: Iterable[Class]) -> None: + # this calls _init_finalbaseobjects for every class and + # create the graph object for the ones that did not raised + # a cycle-error. + self.graph: dict[_ClassOrStr, list[_ClassOrStr]] = {} + self.computed_mros: dict[_ClassOrStr, list[_ClassOrStr]] = {} + + for cls in classes: + try: + self._init_finalbaseobjects(cls) + except ValueError as e: + # Set the MRO right away in case of cycles. + # They should not be in the graph though! + cls.report(str(e), 'mro') + self.computed_mros[cls] = cls._mro = list(cls.allbases(True)) + else: + self.graph[cls] = bases = [] + for b in self._getbases(cls): + bases.append(b) - def getbases(o:Class | str) -> list[Class | str]: - nonlocal _getbases + # string should explicitely be part of the graph + if isinstance(b, str): + self.graph[b] = [] + self.computed_mros[b] = [] - if isinstance(o, str): - return [] + def compute_mros(self) -> None: - if o in _bases: - return _bases[o] + # If this raises a CycleError, our code is boggus since we already + # checked for cycles ourself. + static_order: Iterable[_ClassOrStr] = topsort(self.graph) - r = list(_getbases(o)) - _bases[o] = r + for cls in static_order: + if cls in self.computed_mros: + continue + # All strings bases are already pre-computed to the empty string, + # so the cls varible must be a Class at this point + assert isinstance(cls, Class) + self.computed_mros[cls] = cls._mro = self._compute_mro(cls) - if _has_generic and cleanup_generics: - _getbases = partial(_getbases, ignore_generic=True) - - return r + def _compute_mro(self, cls: Class) -> list[_ClassOrStr]: + """ + Compute the method resolution order for this class. + This assumes that the MRO of the bases of the class + have already been computed and stored in C{self.computed_mros}. + """ + result = [cls] - init_finalbaseobjects(cls) - _mro: list[Class | str] = mro.mro(cls, getbases) + if not (bases:=self.graph[cls]): + return result + + # since we compute all MRO in topoligical orer, we can safely assume + # that self.computed_mros contains all the MROs of the bases of this class. + bases_mros = [self.computed_mros[kls] for kls in bases] + + # handle multiple typing.Generic case, + # see https://github.com/twisted/pydoctor/issues/846. + if 'typing.Generic' in bases and any('typing.Generic' in _mro for _mro in bases_mros): + bases.remove('typing.Generic') + + try: + return result + mro.c3_merge(*bases_mros, bases) + + except ValueError as e: + cls.report(f'{e} of {cls.fullName()!r}', 'mro') + return list(cls.allbases(True)) - if cleanup_generics: - _mro.sort(key=lambda c: c == 'typing.Generic') - return _mro def _find_dunder_constructor(cls:'Class') -> Optional['Function']: """ @@ -744,19 +777,6 @@ def setup(self) -> None: self._initialbases: List[str] = [] self._initialbaseobjects: List[Optional['Class']] = [] - def _init_mro(self) -> None: - """ - Compute the correct value of the method resolution order returned by L{mro()}. - """ - try: - self._mro = compute_mro(self) - except ValueError: - try: - self._mro = compute_mro(self, cleanup_generics=True) - except ValueError as e: - self.report(str(e), 'mro') - self._mro = list(self.allbases(True)) - @overload def mro(self, include_external:'Literal[True]', include_self:bool=True) -> Sequence[Class | str]:... @overload @@ -1541,10 +1561,15 @@ def fetchIntersphinxInventories(self, cache: CacheT) -> None: self.intersphinx.update(cache, url) def defaultPostProcess(system:'System') -> None: - for cls in system.objectsOfType(Class): - # Initiate the MROs - cls._init_mro() + # Init class graph and final bases. + class_finalizer = ClassHierarchyFinalizer( + system.objectsOfType(Class)) + + # Compute MROs + class_finalizer.compute_mros() + + for cls in system.objectsOfType(Class): # Compute subclasses for b in cls.baseobjects: if b is not None: diff --git a/pydoctor/mro.py b/pydoctor/mro.py index e8941e2aa..db63a86bf 100644 --- a/pydoctor/mro.py +++ b/pydoctor/mro.py @@ -103,7 +103,7 @@ def remove(self, item: Optional[T]) -> None: i.popleft() -def _merge(*lists) -> list: +def c3_merge(*lists) -> list: result: List[Optional[T]] = [] linearizations = DependencyList(*lists) @@ -123,14 +123,3 @@ def _merge(*lists) -> list: # Loop never broke, no linearization could possibly be found raise ValueError('Cannot compute linearization of the class inheritance hierarchy') - -def mro(cls: T, getbases: Callable[[T], List[T]]) -> List[T]: - """ - Return a list of classes in order corresponding to Python's MRO. - """ - result = [cls] - - if not getbases(cls): - return result - else: - return result + _merge(*[mro(kls, getbases) for kls in getbases(cls)], getbases(cls)) diff --git a/pydoctor/test/test_mro.py b/pydoctor/test/test_mro.py index 53acb809d..ac221a088 100644 --- a/pydoctor/test/test_mro.py +++ b/pydoctor/test/test_mro.py @@ -11,7 +11,7 @@ def assert_mro_equals(klass: Optional[model.Documentable], expected_mro: List[st assert [member.fullName() if isinstance(member, model.Documentable) else member for member in klass.mro(True)] == expected_mro @systemcls_param -def test_mro(systemcls: Type[model.System],) -> None: +def test_mro(systemcls: Type[model.System], capsys:CapSys) -> None: mod = fromText("""\ from mod import External class C: pass @@ -111,12 +111,11 @@ class GenericPedalo(MyGeneric[ast.AST], Pedalo):... 'mro.WheelBoat', 'mro.Boat']) - with pytest.raises(ValueError, match="Cannot compute linearization"): - model.compute_mro(mod.contents["F1"]) # type:ignore - with pytest.raises(ValueError, match="Cannot compute linearization"): - model.compute_mro(mod.contents["G1"]) # type:ignore - with pytest.raises(ValueError, match="Cannot compute linearization"): - model.compute_mro(mod.contents["Duplicates"]) # type:ignore + assert capsys.readouterr().out == '''mro:31: Cannot compute linearization of the class inheritance hierarchy of 'mro.Duplicates' +mro:9: Cannot compute linearization of the class inheritance hierarchy of 'mro.F1' +mro:10: Cannot compute linearization of the class inheritance hierarchy of 'mro.G1' +''' + def test_mro_cycle(capsys:CapSys) -> None: fromText("""\ @@ -130,8 +129,7 @@ class D(C):... cycle:4: Cycle found while computing inheritance hierarchy: cycle.D -> cycle.C -> cycle.A -> cycle.D ''' -@systemcls_param -def test_mro_generic(systemcls: Type[model.System], capsys:CapSys) -> None: +def test_mro_generic(capsys:CapSys) -> None: src = ''' from typing import Generic, TypeVar T = TypeVar('T') @@ -139,13 +137,26 @@ class A: ... class B(Generic[T]): ... class C(A, Generic[T], B[T]): ... ''' - mod = fromText(src, modname='t', systemcls=systemcls) + mod = fromText(src, modname='t') assert not capsys.readouterr().out assert_mro_equals(mod.contents['C'], ["t.C", "t.A", "t.B", "typing.Generic"]) -@systemcls_param -def test_mro_generic_2(systemcls: Type[model.System], capsys:CapSys) -> None: +def test_mro_generic_4(capsys:CapSys) -> None: + src = ''' + from typing import Generic, TypeVar + T = TypeVar('T') + class A: ... + class Z: ... + class B(Generic[T], Z): ... + class C(A, Generic[T], B[T]): ... + ''' + mod = fromText(src, modname='t') + assert not capsys.readouterr().out + assert_mro_equals(mod.contents['C'], + ["t.C", "t.A", "t.B", "typing.Generic", "t.Z"]) + +def test_mro_generic_2(capsys:CapSys) -> None: src = ''' from typing import Generic, TypeVar T1 = TypeVar('T1') @@ -154,13 +165,12 @@ class A(Generic[T1]): ... class B(Generic[T2]): ... class C(A[T1], B[T2]): ... ''' - mod = fromText(src, modname='t', systemcls=systemcls) + mod = fromText(src, modname='t') assert not capsys.readouterr().out assert_mro_equals(mod.contents['C'], ["t.C", "t.A", "t.B", "typing.Generic"]) -@systemcls_param -def test_mro_generic_3(systemcls: Type[model.System], capsys:CapSys) -> None: +def test_mro_generic_3(capsys:CapSys) -> None: src = ''' from typing import Generic as TGeneric, TypeVar T = TypeVar('T') @@ -169,7 +179,7 @@ class A(Generic): ... class B(TGeneric[T]): ... class C(A, B[T]): ... ''' - mod = fromText(src, modname='t', systemcls=systemcls) + mod = fromText(src, modname='t') assert not capsys.readouterr().out assert_mro_equals(mod.contents['C'], ["t.C", "t.A", "t.Generic", "t.B", "typing.Generic"]) diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py new file mode 100644 index 000000000..0f59f6760 --- /dev/null +++ b/pydoctor/topsort.py @@ -0,0 +1,268 @@ +""" +This module offer a compatibility layer on top of L{graphlib.TopologicalSorter.static_order}. +""" +import sys +from typing import TypeVar, TYPE_CHECKING + +if TYPE_CHECKING: + from typing import TypeAlias + +T = TypeVar('T') +Graph = TypeAlias = dict[T, list[T]] + +if sys.version_info >= (3, 9): + from graphlib import TopologicalSorter, CycleError +else: + # TODO: Remove me when we drop support for python 3.8! + + _NODE_OUT = -1 + _NODE_DONE = -2 + + class _NodeInfo: + __slots__ = "node", "npredecessors", "successors" + + def __init__(self, node): + # The node this class is augmenting. + self.node = node + + # Number of predecessors, generally >= 0. When this value falls to 0, + # and is returned by get_ready(), this is set to _NODE_OUT and when the + # node is marked done by a call to done(), set to _NODE_DONE. + self.npredecessors = 0 + + # List of successor nodes. The list can contain duplicated elements as + # long as they're all reflected in the successor's npredecessors attribute. + self.successors = [] + + class CycleError(ValueError): + """Subclass of ValueError raised by TopologicalSorter.prepare if cycles + exist in the working graph. + + If multiple cycles exist, only one undefined choice among them will be reported + and included in the exception. The detected cycle can be accessed via the second + element in the *args* attribute of the exception instance and consists in a list + of nodes, such that each node is, in the graph, an immediate predecessor of the + next node in the list. In the reported list, the first and the last node will be + the same, to make it clear that it is cyclic. + """ + + pass + + class TopologicalSorter: + """Provides functionality to topologically sort a graph of hashable nodes""" + + def __init__(self, graph=None): + self._node2info = {} + self._ready_nodes = None + self._npassedout = 0 + self._nfinished = 0 + + if graph is not None: + for node, predecessors in graph.items(): + self.add(node, *predecessors) + + def _get_nodeinfo(self, node): + if (result := self._node2info.get(node)) is None: + self._node2info[node] = result = _NodeInfo(node) + return result + + def add(self, node, *predecessors): + """Add a new node and its predecessors to the graph. + + Both the *node* and all elements in *predecessors* must be hashable. + + If called multiple times with the same node argument, the set of dependencies + will be the union of all dependencies passed in. + + It is possible to add a node with no dependencies (*predecessors* is not provided) + as well as provide a dependency twice. If a node that has not been provided before + is included among *predecessors* it will be automatically added to the graph with + no predecessors of its own. + + Raises ValueError if called after "prepare". + """ + if self._ready_nodes is not None: + raise ValueError("Nodes cannot be added after a call to prepare()") + + # Create the node -> predecessor edges + nodeinfo = self._get_nodeinfo(node) + nodeinfo.npredecessors += len(predecessors) + + # Create the predecessor -> node edges + for pred in predecessors: + pred_info = self._get_nodeinfo(pred) + pred_info.successors.append(node) + + def prepare(self): + """Mark the graph as finished and check for cycles in the graph. + + If any cycle is detected, "CycleError" will be raised, but "get_ready" can + still be used to obtain as many nodes as possible until cycles block more + progress. After a call to this function, the graph cannot be modified and + therefore no more nodes can be added using "add". + """ + if self._ready_nodes is not None: + raise ValueError("cannot prepare() more than once") + + self._ready_nodes = [ + i.node for i in self._node2info.values() if i.npredecessors == 0 + ] + # ready_nodes is set before we look for cycles on purpose: + # if the user wants to catch the CycleError, that's fine, + # they can continue using the instance to grab as many + # nodes as possible before cycles block more progress + cycle = self._find_cycle() + if cycle: + raise CycleError(f"nodes are in a cycle", cycle) + + def get_ready(self): + """Return a tuple of all the nodes that are ready. + + Initially it returns all nodes with no predecessors; once those are marked + as processed by calling "done", further calls will return all new nodes that + have all their predecessors already processed. Once no more progress can be made, + empty tuples are returned. + + Raises ValueError if called without calling "prepare" previously. + """ + if self._ready_nodes is None: + raise ValueError("prepare() must be called first") + + # Get the nodes that are ready and mark them + result = tuple(self._ready_nodes) + n2i = self._node2info + for node in result: + n2i[node].npredecessors = _NODE_OUT + + # Clean the list of nodes that are ready and update + # the counter of nodes that we have returned. + self._ready_nodes.clear() + self._npassedout += len(result) + + return result + + def is_active(self): + """Return ``True`` if more progress can be made and ``False`` otherwise. + + Progress can be made if cycles do not block the resolution and either there + are still nodes ready that haven't yet been returned by "get_ready" or the + number of nodes marked "done" is less than the number that have been returned + by "get_ready". + + Raises ValueError if called without calling "prepare" previously. + """ + if self._ready_nodes is None: + raise ValueError("prepare() must be called first") + return self._nfinished < self._npassedout or bool(self._ready_nodes) + + def __bool__(self): + return self.is_active() + + def done(self, *nodes): + """Marks a set of nodes returned by "get_ready" as processed. + + This method unblocks any successor of each node in *nodes* for being returned + in the future by a call to "get_ready". + + Raises :exec:`ValueError` if any node in *nodes* has already been marked as + processed by a previous call to this method, if a node was not added to the + graph by using "add" or if called without calling "prepare" previously or if + node has not yet been returned by "get_ready". + """ + + if self._ready_nodes is None: + raise ValueError("prepare() must be called first") + + n2i = self._node2info + + for node in nodes: + + # Check if we know about this node (it was added previously using add() + if (nodeinfo := n2i.get(node)) is None: + raise ValueError(f"node {node!r} was not added using add()") + + # If the node has not being returned (marked as ready) previously, inform the user. + stat = nodeinfo.npredecessors + if stat != _NODE_OUT: + if stat >= 0: + raise ValueError( + f"node {node!r} was not passed out (still not ready)" + ) + elif stat == _NODE_DONE: + raise ValueError(f"node {node!r} was already marked done") + else: + assert False, f"node {node!r}: unknown status {stat}" + + # Mark the node as processed + nodeinfo.npredecessors = _NODE_DONE + + # Go to all the successors and reduce the number of predecessors, collecting all the ones + # that are ready to be returned in the next get_ready() call. + for successor in nodeinfo.successors: + successor_info = n2i[successor] + successor_info.npredecessors -= 1 + if successor_info.npredecessors == 0: + self._ready_nodes.append(successor) + self._nfinished += 1 + + def _find_cycle(self): + n2i = self._node2info + stack = [] + itstack = [] + seen = set() + node2stacki = {} + + for node in n2i: + if node in seen: + continue + + while True: + if node in seen: + # If we have seen already the node and is in the + # current stack we have found a cycle. + if node in node2stacki: + return stack[node2stacki[node] :] + [node] + # else go on to get next successor + else: + seen.add(node) + itstack.append(iter(n2i[node].successors).__next__) + node2stacki[node] = len(stack) + stack.append(node) + + # Backtrack to the topmost stack entry with + # at least another successor. + while stack: + try: + node = itstack[-1]() + break + except StopIteration: + del node2stacki[stack.pop()] + itstack.pop() + else: + break + return None + + def static_order(self): + """Returns an iterable of nodes in a topological order. + + The particular order that is returned may depend on the specific + order in which the items were inserted in the graph. + + Using this method does not require to call "prepare" or "done". If any + cycle is detected, :exc:`CycleError` will be raised. + """ + self.prepare() + while self.is_active(): + node_group = self.get_ready() + yield from node_group + self.done(*node_group) + +# API + +def topsort(graph: Graph) -> list[T]: + """ + Wrapper for L{graphlib.TopologicalSorter.static_order}. + """ + return TopologicalSorter(graph).static_order() + +__all__ = ['CycleError', 'topsort'] \ No newline at end of file From 645192684ca362c1faf2416b5ca9591d65ae47df Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 18:03:08 -0500 Subject: [PATCH 06/20] Fix pyflakes --- pydoctor/model.py | 1 - pydoctor/mro.py | 2 +- pydoctor/test/test_mro.py | 1 - pydoctor/topsort.py | 4 ++-- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index b89b78e4d..52bb77f53 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -9,7 +9,6 @@ import abc import ast -from functools import partial from itertools import chain from collections import defaultdict import datetime diff --git a/pydoctor/mro.py b/pydoctor/mro.py index db63a86bf..30997712f 100644 --- a/pydoctor/mro.py +++ b/pydoctor/mro.py @@ -27,7 +27,7 @@ from collections import deque from itertools import islice -from typing import Callable, List, Tuple, Optional, TypeVar +from typing import List, Tuple, Optional, TypeVar T = TypeVar('T') diff --git a/pydoctor/test/test_mro.py b/pydoctor/test/test_mro.py index ac221a088..17b910e55 100644 --- a/pydoctor/test/test_mro.py +++ b/pydoctor/test/test_mro.py @@ -1,5 +1,4 @@ from typing import List, Optional, Type -import pytest from pydoctor import model, stanutils from pydoctor.templatewriter import pages, util diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py index 0f59f6760..729a4868e 100644 --- a/pydoctor/topsort.py +++ b/pydoctor/topsort.py @@ -8,7 +8,7 @@ from typing import TypeAlias T = TypeVar('T') -Graph = TypeAlias = dict[T, list[T]] +Graph: TypeAlias = dict[T, list[T]] if sys.version_info >= (3, 9): from graphlib import TopologicalSorter, CycleError @@ -113,7 +113,7 @@ def prepare(self): # nodes as possible before cycles block more progress cycle = self._find_cycle() if cycle: - raise CycleError(f"nodes are in a cycle", cycle) + raise CycleError("nodes are in a cycle", cycle) def get_ready(self): """Return a tuple of all the nodes that are ready. From 6334e282f4f093a90c664f8b7040ce98b3f13708 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 18:03:59 -0500 Subject: [PATCH 07/20] Fix type alias for older python versions --- pydoctor/topsort.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py index 729a4868e..193f06086 100644 --- a/pydoctor/topsort.py +++ b/pydoctor/topsort.py @@ -8,7 +8,7 @@ from typing import TypeAlias T = TypeVar('T') -Graph: TypeAlias = dict[T, list[T]] +Graph: TypeAlias = 'dict[T, list[T]]' if sys.version_info >= (3, 9): from graphlib import TopologicalSorter, CycleError From d2df6657c93570a1f9b2e52d52fedb99e1138774 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 18:07:33 -0500 Subject: [PATCH 08/20] Forgot the __future__ import --- pydoctor/topsort.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py index 193f06086..8b71fd6bd 100644 --- a/pydoctor/topsort.py +++ b/pydoctor/topsort.py @@ -1,6 +1,8 @@ """ This module offer a compatibility layer on top of L{graphlib.TopologicalSorter.static_order}. """ +from __future__ import annotations + import sys from typing import TypeVar, TYPE_CHECKING From 7a23766b1c7563d31fd938ac7054c70e0a1c03af Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 18:09:47 -0500 Subject: [PATCH 09/20] Fix mypy --- pydoctor/model.py | 2 +- pydoctor/topsort.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 52bb77f53..c73cf58a4 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -686,7 +686,7 @@ def _compute_mro(self, cls: Class) -> list[_ClassOrStr]: This assumes that the MRO of the bases of the class have already been computed and stored in C{self.computed_mros}. """ - result = [cls] + result: list[_ClassOrStr] = [cls] if not (bases:=self.graph[cls]): return result diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py index 8b71fd6bd..fb6f31d43 100644 --- a/pydoctor/topsort.py +++ b/pydoctor/topsort.py @@ -4,7 +4,7 @@ from __future__ import annotations import sys -from typing import TypeVar, TYPE_CHECKING +from typing import Iterable, TypeVar, TYPE_CHECKING if TYPE_CHECKING: from typing import TypeAlias @@ -261,7 +261,7 @@ def static_order(self): # API -def topsort(graph: Graph) -> list[T]: +def topsort(graph: Graph[T]) -> Iterable[T]: """ Wrapper for L{graphlib.TopologicalSorter.static_order}. """ From 283d3403ab7d296bee80317b8cbda66e45022db4 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Sun, 8 Dec 2024 23:54:08 -0500 Subject: [PATCH 10/20] Try a simpler alternative for python 3.8 --- pydoctor/model.py | 45 ++++++-- pydoctor/topsort.py | 270 -------------------------------------------- 2 files changed, 35 insertions(+), 280 deletions(-) delete mode 100644 pydoctor/topsort.py diff --git a/pydoctor/model.py b/pydoctor/model.py index c73cf58a4..a7fdc0cd9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -32,7 +32,6 @@ from pydoctor import factory, qnmatch, utils, linker, astutils, mro from pydoctor.epydoc.markup import ParsedDocstring from pydoctor.sphinx import CacheT, SphinxInventory -from pydoctor.topsort import topsort if TYPE_CHECKING: from typing import Literal, Protocol, TypeAlias @@ -41,6 +40,7 @@ Literal = {True: bool, False: bool} ASTBuilder = Protocol = object +T = TypeVar('T') # originally when I started to write pydoctor I had this idea of a big # tree of Documentables arranged in an almost arbitrary tree. @@ -583,7 +583,34 @@ def is_exception(cls: 'Class') -> bool: return True return False -_ClassOrStr: TypeAlias = 'Class | str' +Graph: TypeAlias = 'dict[T, list[T]]' + +if sys.version_info >= (3, 9): + from graphlib import TopologicalSorter + def topsort(graph: Graph[T]) -> Iterable[T]: + """ + Wrapper for L{graphlib.TopologicalSorter.static_order}. + """ + return TopologicalSorter(graph).static_order() +else: + from collections import deque + def topsort(graph: Graph[T]) -> Iterable[T]: + result = deque() + visited = set() + stack = [[key for key in graph]] + while stack: + for i in stack[-1]: + if i in visited and i not in result: + result.appendleft(i) + if i not in visited: + visited.add(i) + stack.append(graph[i]) + break + else: + stack.pop() + return result + +ClassOrStr: TypeAlias = 'Class | str' class ClassHierarchyFinalizer: """ @@ -630,7 +657,7 @@ def _init_finalbaseobjects(o: Class, path:list[Class] | None = None) -> None: o._finalbases = finalbases @staticmethod - def _getbases(o: Class) -> Iterator[_ClassOrStr]: + def _getbases(o: Class) -> Iterator[ClassOrStr]: """ Like L{Class.baseobjects} but fallback to the expanded name if the base is not resolved to a L{Class} object. @@ -645,8 +672,8 @@ def __init__(self, classes: Iterable[Class]) -> None: # this calls _init_finalbaseobjects for every class and # create the graph object for the ones that did not raised # a cycle-error. - self.graph: dict[_ClassOrStr, list[_ClassOrStr]] = {} - self.computed_mros: dict[_ClassOrStr, list[_ClassOrStr]] = {} + self.graph: dict[ClassOrStr, list[ClassOrStr]] = {} + self.computed_mros: dict[ClassOrStr, list[ClassOrStr]] = {} for cls in classes: try: @@ -670,7 +697,7 @@ def compute_mros(self) -> None: # If this raises a CycleError, our code is boggus since we already # checked for cycles ourself. - static_order: Iterable[_ClassOrStr] = topsort(self.graph) + static_order: Iterable[ClassOrStr] = topsort(self.graph) for cls in static_order: if cls in self.computed_mros: @@ -680,13 +707,13 @@ def compute_mros(self) -> None: assert isinstance(cls, Class) self.computed_mros[cls] = cls._mro = self._compute_mro(cls) - def _compute_mro(self, cls: Class) -> list[_ClassOrStr]: + def _compute_mro(self, cls: Class) -> list[ClassOrStr]: """ Compute the method resolution order for this class. This assumes that the MRO of the bases of the class have already been computed and stored in C{self.computed_mros}. """ - result: list[_ClassOrStr] = [cls] + result: list[ClassOrStr] = [cls] if not (bases:=self.graph[cls]): return result @@ -951,8 +978,6 @@ class Attribute(Inheritable): _ModuleT = Module _PackageT = Package -T = TypeVar('T') - def import_mod_from_file_location(module_full_name:str, path: Path) -> types.ModuleType: spec = importlib.util.spec_from_file_location(module_full_name, path) if spec is None: diff --git a/pydoctor/topsort.py b/pydoctor/topsort.py deleted file mode 100644 index fb6f31d43..000000000 --- a/pydoctor/topsort.py +++ /dev/null @@ -1,270 +0,0 @@ -""" -This module offer a compatibility layer on top of L{graphlib.TopologicalSorter.static_order}. -""" -from __future__ import annotations - -import sys -from typing import Iterable, TypeVar, TYPE_CHECKING - -if TYPE_CHECKING: - from typing import TypeAlias - -T = TypeVar('T') -Graph: TypeAlias = 'dict[T, list[T]]' - -if sys.version_info >= (3, 9): - from graphlib import TopologicalSorter, CycleError -else: - # TODO: Remove me when we drop support for python 3.8! - - _NODE_OUT = -1 - _NODE_DONE = -2 - - class _NodeInfo: - __slots__ = "node", "npredecessors", "successors" - - def __init__(self, node): - # The node this class is augmenting. - self.node = node - - # Number of predecessors, generally >= 0. When this value falls to 0, - # and is returned by get_ready(), this is set to _NODE_OUT and when the - # node is marked done by a call to done(), set to _NODE_DONE. - self.npredecessors = 0 - - # List of successor nodes. The list can contain duplicated elements as - # long as they're all reflected in the successor's npredecessors attribute. - self.successors = [] - - class CycleError(ValueError): - """Subclass of ValueError raised by TopologicalSorter.prepare if cycles - exist in the working graph. - - If multiple cycles exist, only one undefined choice among them will be reported - and included in the exception. The detected cycle can be accessed via the second - element in the *args* attribute of the exception instance and consists in a list - of nodes, such that each node is, in the graph, an immediate predecessor of the - next node in the list. In the reported list, the first and the last node will be - the same, to make it clear that it is cyclic. - """ - - pass - - class TopologicalSorter: - """Provides functionality to topologically sort a graph of hashable nodes""" - - def __init__(self, graph=None): - self._node2info = {} - self._ready_nodes = None - self._npassedout = 0 - self._nfinished = 0 - - if graph is not None: - for node, predecessors in graph.items(): - self.add(node, *predecessors) - - def _get_nodeinfo(self, node): - if (result := self._node2info.get(node)) is None: - self._node2info[node] = result = _NodeInfo(node) - return result - - def add(self, node, *predecessors): - """Add a new node and its predecessors to the graph. - - Both the *node* and all elements in *predecessors* must be hashable. - - If called multiple times with the same node argument, the set of dependencies - will be the union of all dependencies passed in. - - It is possible to add a node with no dependencies (*predecessors* is not provided) - as well as provide a dependency twice. If a node that has not been provided before - is included among *predecessors* it will be automatically added to the graph with - no predecessors of its own. - - Raises ValueError if called after "prepare". - """ - if self._ready_nodes is not None: - raise ValueError("Nodes cannot be added after a call to prepare()") - - # Create the node -> predecessor edges - nodeinfo = self._get_nodeinfo(node) - nodeinfo.npredecessors += len(predecessors) - - # Create the predecessor -> node edges - for pred in predecessors: - pred_info = self._get_nodeinfo(pred) - pred_info.successors.append(node) - - def prepare(self): - """Mark the graph as finished and check for cycles in the graph. - - If any cycle is detected, "CycleError" will be raised, but "get_ready" can - still be used to obtain as many nodes as possible until cycles block more - progress. After a call to this function, the graph cannot be modified and - therefore no more nodes can be added using "add". - """ - if self._ready_nodes is not None: - raise ValueError("cannot prepare() more than once") - - self._ready_nodes = [ - i.node for i in self._node2info.values() if i.npredecessors == 0 - ] - # ready_nodes is set before we look for cycles on purpose: - # if the user wants to catch the CycleError, that's fine, - # they can continue using the instance to grab as many - # nodes as possible before cycles block more progress - cycle = self._find_cycle() - if cycle: - raise CycleError("nodes are in a cycle", cycle) - - def get_ready(self): - """Return a tuple of all the nodes that are ready. - - Initially it returns all nodes with no predecessors; once those are marked - as processed by calling "done", further calls will return all new nodes that - have all their predecessors already processed. Once no more progress can be made, - empty tuples are returned. - - Raises ValueError if called without calling "prepare" previously. - """ - if self._ready_nodes is None: - raise ValueError("prepare() must be called first") - - # Get the nodes that are ready and mark them - result = tuple(self._ready_nodes) - n2i = self._node2info - for node in result: - n2i[node].npredecessors = _NODE_OUT - - # Clean the list of nodes that are ready and update - # the counter of nodes that we have returned. - self._ready_nodes.clear() - self._npassedout += len(result) - - return result - - def is_active(self): - """Return ``True`` if more progress can be made and ``False`` otherwise. - - Progress can be made if cycles do not block the resolution and either there - are still nodes ready that haven't yet been returned by "get_ready" or the - number of nodes marked "done" is less than the number that have been returned - by "get_ready". - - Raises ValueError if called without calling "prepare" previously. - """ - if self._ready_nodes is None: - raise ValueError("prepare() must be called first") - return self._nfinished < self._npassedout or bool(self._ready_nodes) - - def __bool__(self): - return self.is_active() - - def done(self, *nodes): - """Marks a set of nodes returned by "get_ready" as processed. - - This method unblocks any successor of each node in *nodes* for being returned - in the future by a call to "get_ready". - - Raises :exec:`ValueError` if any node in *nodes* has already been marked as - processed by a previous call to this method, if a node was not added to the - graph by using "add" or if called without calling "prepare" previously or if - node has not yet been returned by "get_ready". - """ - - if self._ready_nodes is None: - raise ValueError("prepare() must be called first") - - n2i = self._node2info - - for node in nodes: - - # Check if we know about this node (it was added previously using add() - if (nodeinfo := n2i.get(node)) is None: - raise ValueError(f"node {node!r} was not added using add()") - - # If the node has not being returned (marked as ready) previously, inform the user. - stat = nodeinfo.npredecessors - if stat != _NODE_OUT: - if stat >= 0: - raise ValueError( - f"node {node!r} was not passed out (still not ready)" - ) - elif stat == _NODE_DONE: - raise ValueError(f"node {node!r} was already marked done") - else: - assert False, f"node {node!r}: unknown status {stat}" - - # Mark the node as processed - nodeinfo.npredecessors = _NODE_DONE - - # Go to all the successors and reduce the number of predecessors, collecting all the ones - # that are ready to be returned in the next get_ready() call. - for successor in nodeinfo.successors: - successor_info = n2i[successor] - successor_info.npredecessors -= 1 - if successor_info.npredecessors == 0: - self._ready_nodes.append(successor) - self._nfinished += 1 - - def _find_cycle(self): - n2i = self._node2info - stack = [] - itstack = [] - seen = set() - node2stacki = {} - - for node in n2i: - if node in seen: - continue - - while True: - if node in seen: - # If we have seen already the node and is in the - # current stack we have found a cycle. - if node in node2stacki: - return stack[node2stacki[node] :] + [node] - # else go on to get next successor - else: - seen.add(node) - itstack.append(iter(n2i[node].successors).__next__) - node2stacki[node] = len(stack) - stack.append(node) - - # Backtrack to the topmost stack entry with - # at least another successor. - while stack: - try: - node = itstack[-1]() - break - except StopIteration: - del node2stacki[stack.pop()] - itstack.pop() - else: - break - return None - - def static_order(self): - """Returns an iterable of nodes in a topological order. - - The particular order that is returned may depend on the specific - order in which the items were inserted in the graph. - - Using this method does not require to call "prepare" or "done". If any - cycle is detected, :exc:`CycleError` will be raised. - """ - self.prepare() - while self.is_active(): - node_group = self.get_ready() - yield from node_group - self.done(*node_group) - -# API - -def topsort(graph: Graph[T]) -> Iterable[T]: - """ - Wrapper for L{graphlib.TopologicalSorter.static_order}. - """ - return TopologicalSorter(graph).static_order() - -__all__ = ['CycleError', 'topsort'] \ No newline at end of file From 88f5613ee232939ed40d3316ba43996d294ce7d8 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 9 Dec 2024 00:01:17 -0500 Subject: [PATCH 11/20] Give-up on python 3.8 for this fix --- pydoctor/model.py | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index a7fdc0cd9..437c6757d 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -24,6 +24,7 @@ TYPE_CHECKING, Any, Collection, Dict, Iterable, Iterator, List, Mapping, Callable, Optional, Sequence, Set, Tuple, Type, TypeVar, Union, cast, overload ) +from graphlib import TopologicalSorter from urllib.parse import quote import attr @@ -585,30 +586,11 @@ def is_exception(cls: 'Class') -> bool: Graph: TypeAlias = 'dict[T, list[T]]' -if sys.version_info >= (3, 9): - from graphlib import TopologicalSorter - def topsort(graph: Graph[T]) -> Iterable[T]: - """ - Wrapper for L{graphlib.TopologicalSorter.static_order}. - """ - return TopologicalSorter(graph).static_order() -else: - from collections import deque - def topsort(graph: Graph[T]) -> Iterable[T]: - result = deque() - visited = set() - stack = [[key for key in graph]] - while stack: - for i in stack[-1]: - if i in visited and i not in result: - result.appendleft(i) - if i not in visited: - visited.add(i) - stack.append(graph[i]) - break - else: - stack.pop() - return result +def topsort(graph: Graph[T]) -> Iterable[T]: + """ + Wrapper for L{graphlib.TopologicalSorter.static_order}. + """ + return TopologicalSorter(graph).static_order() ClassOrStr: TypeAlias = 'Class | str' @@ -724,8 +706,10 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: # handle multiple typing.Generic case, # see https://github.com/twisted/pydoctor/issues/846. - if 'typing.Generic' in bases and any('typing.Generic' in _mro for _mro in bases_mros): - bases.remove('typing.Generic') + # support documenting typing.py module by using allobject.get. + generic = cls.system.allobjects.get(d:='typing.Generic', d) + if generic in bases and any(generic in _mro for _mro in bases_mros): + bases.remove(generic) try: return result + mro.c3_merge(*bases_mros, bases) From 0b6fa3a6e6f38f6d3aa2538f78d889343780d9a4 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:46:08 -0500 Subject: [PATCH 12/20] Apply suggestions from code review --- pydoctor/model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 437c6757d..0649da901 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -684,7 +684,7 @@ def compute_mros(self) -> None: for cls in static_order: if cls in self.computed_mros: continue - # All strings bases are already pre-computed to the empty string, + # All strings bases are already pre-computed to the empty list, # so the cls varible must be a Class at this point assert isinstance(cls, Class) self.computed_mros[cls] = cls._mro = self._compute_mro(cls) @@ -700,7 +700,7 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: if not (bases:=self.graph[cls]): return result - # since we compute all MRO in topoligical orer, we can safely assume + # since we compute all MRO in topological order, we can safely assume # that self.computed_mros contains all the MROs of the bases of this class. bases_mros = [self.computed_mros[kls] for kls in bases] From c838a655d6e6cbab32c0fa5873d6c6eecdb091f9 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Mon, 9 Dec 2024 13:51:34 -0500 Subject: [PATCH 13/20] Babysit mypy --- pydoctor/model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 0649da901..eeac34b10 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -709,7 +709,8 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: # support documenting typing.py module by using allobject.get. generic = cls.system.allobjects.get(d:='typing.Generic', d) if generic in bases and any(generic in _mro for _mro in bases_mros): - bases.remove(generic) + # this is cafe since we checked 'generic in bases'. + bases.remove(generic) # type: ignore[arg-type] try: return result + mro.c3_merge(*bases_mros, bases) From 7c772fe4fc22fffb0aec57ab909714fe2018ccc0 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Mon, 9 Dec 2024 14:08:56 -0500 Subject: [PATCH 14/20] Fix typo --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index eeac34b10..c59e1ff6f 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -685,7 +685,7 @@ def compute_mros(self) -> None: if cls in self.computed_mros: continue # All strings bases are already pre-computed to the empty list, - # so the cls varible must be a Class at this point + # so the cls variable must be a Class at this point assert isinstance(cls, Class) self.computed_mros[cls] = cls._mro = self._compute_mro(cls) From 2b8eb368fa1644c9126c2f05f17a92329d22c70e Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Mon, 9 Dec 2024 15:50:25 -0500 Subject: [PATCH 15/20] Apply suggestions from code review --- pydoctor/model.py | 2 +- pydoctor/test/test_mro.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index c59e1ff6f..bce13e8d7 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -709,7 +709,7 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: # support documenting typing.py module by using allobject.get. generic = cls.system.allobjects.get(d:='typing.Generic', d) if generic in bases and any(generic in _mro for _mro in bases_mros): - # this is cafe since we checked 'generic in bases'. + # this is safe since we checked 'generic in bases'. bases.remove(generic) # type: ignore[arg-type] try: diff --git a/pydoctor/test/test_mro.py b/pydoctor/test/test_mro.py index 17b910e55..1f2e5a41a 100644 --- a/pydoctor/test/test_mro.py +++ b/pydoctor/test/test_mro.py @@ -141,6 +141,21 @@ class C(A, Generic[T], B[T]): ... assert_mro_equals(mod.contents['C'], ["t.C", "t.A", "t.B", "typing.Generic"]) +def test_mro_generic_in_system(capsys:CapSys) -> None: + src = ''' + class TypeVar:... + class Generic: ... + T = TypeVar('T') + class A: ... + class B(Generic[T]): ... + class C(A, Generic[T], B[T]): ... + ''' + mod = fromText(src, modname='typing') + assert not capsys.readouterr().out + assert_mro_equals(mod.contents['C'], + ["typing.C", "typing.A", "typing.B", "typing.Generic"]) + + def test_mro_generic_4(capsys:CapSys) -> None: src = ''' from typing import Generic, TypeVar From 8d46b2b966d31fce6c35830d738d126b7493de84 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 12 Dec 2024 10:20:44 -0500 Subject: [PATCH 16/20] Do not strore strings in the graph --- pydoctor/model.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index eeac34b10..fbae88004 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -654,7 +654,7 @@ def __init__(self, classes: Iterable[Class]) -> None: # this calls _init_finalbaseobjects for every class and # create the graph object for the ones that did not raised # a cycle-error. - self.graph: dict[ClassOrStr, list[ClassOrStr]] = {} + self.graph: dict[Class, list[ClassOrStr]] = {} self.computed_mros: dict[ClassOrStr, list[ClassOrStr]] = {} for cls in classes: @@ -669,12 +669,9 @@ def __init__(self, classes: Iterable[Class]) -> None: self.graph[cls] = bases = [] for b in self._getbases(cls): bases.append(b) - - # string should explicitely be part of the graph - if isinstance(b, str): - self.graph[b] = [] - self.computed_mros[b] = [] - + # strings are not part of the graph + # because they always have the same MRO: empty list. + def compute_mros(self) -> None: # If this raises a CycleError, our code is boggus since we already @@ -682,11 +679,9 @@ def compute_mros(self) -> None: static_order: Iterable[ClassOrStr] = topsort(self.graph) for cls in static_order: - if cls in self.computed_mros: + if cls in self.computed_mros or isinstance(cls, str): + # If it's already computed, it means it's boggus like with cycle or something. continue - # All strings bases are already pre-computed to the empty list, - # so the cls varible must be a Class at this point - assert isinstance(cls, Class) self.computed_mros[cls] = cls._mro = self._compute_mro(cls) def _compute_mro(self, cls: Class) -> list[ClassOrStr]: @@ -695,6 +690,9 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: This assumes that the MRO of the bases of the class have already been computed and stored in C{self.computed_mros}. """ + if cls not in self.graph: + raise ValueError + result: list[ClassOrStr] = [cls] if not (bases:=self.graph[cls]): @@ -702,12 +700,12 @@ def _compute_mro(self, cls: Class) -> list[ClassOrStr]: # since we compute all MRO in topological order, we can safely assume # that self.computed_mros contains all the MROs of the bases of this class. - bases_mros = [self.computed_mros[kls] for kls in bases] + bases_mros = [self.computed_mros.get(kls, []) for kls in bases] # handle multiple typing.Generic case, # see https://github.com/twisted/pydoctor/issues/846. # support documenting typing.py module by using allobject.get. - generic = cls.system.allobjects.get(d:='typing.Generic', d) + generic = cls.system.allobjects.get(_d:='typing.Generic', _d) if generic in bases and any(generic in _mro for _mro in bases_mros): # this is cafe since we checked 'generic in bases'. bases.remove(generic) # type: ignore[arg-type] From 3baa0c9939d2b74ed13bb9bf584a78cbd9cf36c2 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 12 Dec 2024 11:13:48 -0500 Subject: [PATCH 17/20] Fix typing --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index a92b01071..236c2ea13 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -577,7 +577,7 @@ def is_exception(cls: 'Class') -> bool: return True return False -Graph: TypeAlias = 'dict[T, list[T]]' +Graph: TypeAlias = 'dict[object, list[T]]' def topsort(graph: Graph[T]) -> Iterable[T]: """ From bb4497c87703e91d13a138cfea06653bb89c8378 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 12 Dec 2024 11:16:11 -0500 Subject: [PATCH 18/20] Try to fix typing again --- pydoctor/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 236c2ea13..0a4c7da40 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -577,7 +577,7 @@ def is_exception(cls: 'Class') -> bool: return True return False -Graph: TypeAlias = 'dict[object, list[T]]' +Graph: TypeAlias = 'dict[Any, Sequence[T]]' def topsort(graph: Graph[T]) -> Iterable[T]: """ From 431a976ba5ddd58886d740994dcee7baeff7cdd4 Mon Sep 17 00:00:00 2001 From: tristanlatr Date: Thu, 12 Dec 2024 13:45:39 -0500 Subject: [PATCH 19/20] Try to fix mypy --- pydoctor/model.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 0a4c7da40..11bc62972 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -577,11 +577,12 @@ def is_exception(cls: 'Class') -> bool: return True return False -Graph: TypeAlias = 'dict[Any, Sequence[T]]' - -def topsort(graph: Graph[T]) -> Iterable[T]: +def topsort(graph: Mapping[Any, Sequence[T]]) -> Iterable[T]: """ - Wrapper for L{graphlib.TopologicalSorter.static_order}. + Given a mapping where each keys corespond to a node + and keys the predecessors of the node, return the topological order of the nodes. + + This is a simpple wrapper for L{graphlib.TopologicalSorter.static_order}. """ return TopologicalSorter(graph).static_order() @@ -669,7 +670,7 @@ def compute_mros(self) -> None: # If this raises a CycleError, our code is boggus since we already # checked for cycles ourself. - static_order: Iterable[ClassOrStr] = topsort(self.graph) + static_order = topsort(self.graph) for cls in static_order: if cls in self.computed_mros or isinstance(cls, str): From 0f324295a8cbd2c2728c35ce8788aaf272290205 Mon Sep 17 00:00:00 2001 From: tristanlatr <19967168+tristanlatr@users.noreply.github.com> Date: Thu, 12 Dec 2024 17:47:30 -0500 Subject: [PATCH 20/20] Apply suggestions from code review --- pydoctor/model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pydoctor/model.py b/pydoctor/model.py index 11bc62972..720fffbf9 100644 --- a/pydoctor/model.py +++ b/pydoctor/model.py @@ -579,8 +579,8 @@ def is_exception(cls: 'Class') -> bool: def topsort(graph: Mapping[Any, Sequence[T]]) -> Iterable[T]: """ - Given a mapping where each keys corespond to a node - and keys the predecessors of the node, return the topological order of the nodes. + Given a mapping where each key-value pair correspond to a node and it's + predecessors, return the topological order of the nodes. This is a simpple wrapper for L{graphlib.TopologicalSorter.static_order}. """ @@ -674,7 +674,7 @@ def compute_mros(self) -> None: for cls in static_order: if cls in self.computed_mros or isinstance(cls, str): - # If it's already computed, it means it's boggus like with cycle or something. + # If it's already computed, it means it's bogus continue self.computed_mros[cls] = cls._mro = self._compute_mro(cls)