Skip to content

Conversation

@edreamleo
Copy link
Contributor

@edreamleo edreamleo commented Feb 12, 2023

This PR collapses the complexity of several type-inference classes:

  • Replace all three Abstract classes with one new arg to PyObject.__init__.
  • Use single inheritance for most classes derived from PyObject or PyDefinedObject.
  • PyDefinedObject is a subclass of PyObject.
    The annotation, Union[PyObject, PyDefinedObject], can be replaced by PyObject.

Note: The ekr-mypy-single-inheritance branch (PR #665) annotates this branch, with the usual changes to rope.base.ast needed to let mypy do complete checking. Those checks found three missing functions that have now been moved from two Abstract classes into the appropriate classes in pyobjects.py.

Summary of the changes, in the order they occurred:

  • Simplify the get_base_type and get_unknown functions in pyobjects.py.
  • Remove unnecessary methods from the three Abstract classes.
  • Eliminate the AbstractClass, AbstractFunction, and AbstractModule classes.
  • Use single inheritance for all important type-inference classes.

Step 1: Simplify the get_base_type and get_unknown functions

  • Simplify the get_base_type and get_unknown functions in pyobjects.py.
  • Remove a faux helper: the PyObject._get_base_type static method.

Step 2: Remove most methods from the Abstract classes

  • Remove most methods from the three Abstract classes, retaining 3 methods found nowhere else.
  • Move dummy get_superclasses from the BuiltinObject to its base class, BuiltinClass.
    With this change, BuiltinObject has the same form as the three Abstract classes.

Step 3: Remove all three Abstract classes

  • Add rope/base/utils/predicates.py.
  • Replace all tests isinstance(obj, Abstract*) with is_abstract_*(obj).
    Note: Neither the get_base_type nor the get_unknown function requires the Abstract classes.
  • Remove the three Abstract classes entirely.

Discussion:

  • Rope's inference uses the three Abstract classes as markers.
    The legacy pattern is if isinstance(obj, AbstractX):
    The new pattern is if is_abstract_x(obj):. This pattern is shorter, but is less explicit.
  • The Abstract classes help init their PyObject base classes, but Part 4 shows that this is "faux help".
    The formerly "abstract" classes need only add one new arg when instantiating PyObject.

Step 4: Use single inheritance for all type-inference classes

This step was unexpectedly very easy.

  • PyDefinedObject now has PyObject as its only base class.
    The PyObject annotation can replace Union[PyObject, PyDefinedObject].
  • The following classes (in pyobjects.py) now have PyDefinedObject as their only base class:
    po.PyClass, po.PyComprehension, po.PyFunction, po._PyModule.
  • The following classes (in pyobjectsdef.py) now have only a single base class:
    pod.PyFunction, pod.PyClass, pod.PyComprehension

@edreamleo edreamleo marked this pull request as draft February 12, 2023 19:15
@edreamleo edreamleo changed the title PR: (Experimental) Replace Abstract classes with predicates PR: (Permanent draft) Replace Abstract classes with predicates Feb 13, 2023
@edreamleo edreamleo changed the title PR: (Permanent draft) Replace Abstract classes with predicates PR: Replace Abstract classes with single inheritance Feb 13, 2023
@edreamleo edreamleo changed the title PR: Replace Abstract classes with single inheritance PR: Simplify Rope's class hierarchy Feb 13, 2023
@edreamleo edreamleo changed the title PR: Simplify Rope's class hierarchy PR: (Experimental) Simplify Rope's class hierarchy Feb 13, 2023
@edreamleo
Copy link
Contributor Author

@lieryan I am going to close this PR.

I'll use ekr-fork-rope2 to work on the Theory of Operation without bothering you.

@edreamleo edreamleo closed this Feb 20, 2023
@edreamleo edreamleo deleted the ekr-abstract-predicates branch February 20, 2023 16:33
@edreamleo edreamleo changed the title PR: (Experimental) Simplify Rope's class hierarchy PR: Simplify Rope's class hierarchy Feb 27, 2023
@edreamleo edreamleo restored the ekr-abstract-predicates branch March 5, 2023 15:10
@edreamleo
Copy link
Contributor Author

@lieryan I am going to reopen this PR as a draft. This PR is one of three approaches I am considering:

PR #664 (This PR):Replace Abstract classes with predicate functions defined in pyobjects.py. Changes 14 files.
PR #677: Use empty Abstract classes, keeping everything else the same. Changes 2 files.
PR #nnn: Replace Abstract classes with predicate methods of the PyObject class.

PRs #664 and #677 both work. PR #677 has zero impact on performance. The other two PRs likely have negligible impact on performance, but more testing is needed.

@edreamleo edreamleo marked this pull request as ready for review March 6, 2023 15:04
@edreamleo edreamleo closed this Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant