Meta-ticket: Replace `is_Class` functions, create abstract base classes to enable modularization

mkoeppe commented 3 years ago

git grep 'import.* is_[A-Z]' (or git grep 'import.*is_[A-Z]' | grep -E -v 'is_(Ring|Matrix)[^A-Z]' | grep -v 'sage:') reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module. This pattern is an obstacle to modularization.

The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and isinstance does not help in this regard by itself: The module providing the class must be installed so that the import does not fail.

This can of course be worked around by try... except around import, as done for example in #32455, but we should avoid cluttering the code with this type of workaround.

Instead we propose to create stub classes (abstract base classes) in sage.structure.element, sage.structure.parent, and/or sage.PAC.KAGE.abc, and make the actual implementation classes subclasses (mostly, unique direct subclasses). This pattern already exists in the Sage code for a few existing classes:

sage.structure.element defines a base class Matrix (for purposes of the coercion system) and a function is_Matrix (see #32347)
sage.structure.parent defines a base class Set_generic; using it with isinstance has replaced use of the earlier is_Set function (#24443, #28470)

We add the following new classes:

32566/#32610/#32612: sage.rings.abc defines base class RealDoubleField for sage.rings.real_double.RealDoubleField_class etc.
32606
32660
32750
32638/#32665: in sage.structure.element, a class Expression
- the existing class sage.symbolic.expression.Expression will be its only subclass
- deprecate is_Expression, is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable; replace uses by isinstance(x, Expression) and a method call (may need a new method is_callable)
32637: in sage.geometry.abc, a class Polyhedron
- Polyhedron_base will be its only direct subclass
32709
34804
32664
34931
24525

Technical limitation: If a sage.PAC.KAGE.abc module is intended to provide the new classes, then sage.PAC.KAGE must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out __init__.py files in preparation for namespace packages)

As of this ticket, each of the new abstract base classes is intended to have a unique direct implementation subclass. #32637 adds doctests to document and enforce this design. Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

Tickets not needed for modularization, but removing is_ functions in favor of isinstance for uniformity of the codebase:

32738
34307
37895
37896
37897
37898
37922
37924
38022
38032
38035
38077
38103
38124
38125
38126
38128
38184
38266
to be done: see https://groups.google.com/g/sage-devel/c/6MJHDicPYWs/m/RBxTi4TGAwAJ

Functions that look like is_Class:

38287
38297

38228
38240

Part of:

29705

CC: @tscrim @fchapoton @kliem

Component: refactoring

Issue created by migration from https://trac.sagemath.org/ticket/32414

kliem commented 3 years ago

comment:1

I find this in general bothering that there are tons of modules etc. being imported (even at startup), just because some isinstance check.

The best I can come up with for the second is the following:

_Polyhedron_base = None

class _dummy:
    pass

def is_polyhedron(P):
    global _Polyhedron_base
    if _Polyhedron_base is not None:
        return isinstance(P, _Polyhedron_base)
    try:
        from sage.geometry.polyhedron.base import Polyhedron_base
        _Polyhedron_base = Polyhedron_base
        return isinstance(P, Polyhedron_base)
    except ImportError:
        _Polyhedron_base = _dummy
        return False

Apparently its a slow down of about 50 percent, when the import is found, otherwise much more.

The first thing is even faster (isinstance(P, Polyhedron_containment_check) is about 75 percent of isinstance(P, Polyhedron_base)), but I don't know how much another stupid inheritance slows down the actual class.

I would prefer a unique name scheme that can be used throughout sage. _base is already in use. Could as well be the original class name with a leading underscore to discourage people to use it for anything else, e.g. _Polyhedron_base, _Expression.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,11 +1,13 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.

-This pattern is an obstacle to modularization. (The recent trend to replace this pattern by importing the class and `isinstance` does not help in this regard.)
+This pattern is an obstacle to modularization. (The ongoing effort (#12824, ..., #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)

 We propose to move `is_...` methods into separate Python/Cython modules that can be imported even if the underlying implementation is not present.  For example, if `sagemath-symbolics` is not present, then `is_Expression` should just return `False` for every input.

 Possible implementation strategies:
 - Introduce stub classes, make the actual implementation classes subclasses. For example, define class `Expression_base`; make `Expression` a subclass; `is_Expression(x)` tests `isinstance(x, Expression_base`
+  - `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
+
 - `try: import ... except ImportError: return False`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,12 +1,13 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.

-This pattern is an obstacle to modularization. (The ongoing effort (#12824, ..., #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)
+This pattern is an obstacle to modularization. (The ongoing effort (#12824, ..., #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)

 We propose to move `is_...` methods into separate Python/Cython modules that can be imported even if the underlying implementation is not present.  For example, if `sagemath-symbolics` is not present, then `is_Expression` should just return `False` for every input.

 Possible implementation strategies:
 - Introduce stub classes, make the actual implementation classes subclasses. For example, define class `Expression_base`; make `Expression` a subclass; `is_Expression(x)` tests `isinstance(x, Expression_base`
   - `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
+  - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 - `try: import ... except ImportError: return False`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.

-This pattern is an obstacle to modularization. (The ongoing effort (#12824, ..., #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)
+This pattern is an obstacle to modularization. (The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, ..., #24443, #28470, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)

 We propose to move `is_...` methods into separate Python/Cython modules that can be imported even if the underlying implementation is not present.  For example, if `sagemath-symbolics` is not present, then `is_Expression` should just return `False` for every input.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.

-This pattern is an obstacle to modularization. (The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, ..., #24443, #28470, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)
+This pattern is an obstacle to modularization. (The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)

 We propose to move `is_...` methods into separate Python/Cython modules that can be imported even if the underlying implementation is not present.  For example, if `sagemath-symbolics` is not present, then `is_Expression` should just return `False` for every input.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,14 +1,19 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.

-This pattern is an obstacle to modularization. (The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.)
+This pattern is an obstacle to modularization.

-We propose to move `is_...` methods into separate Python/Cython modules that can be imported even if the underlying implementation is not present.  For example, if `sagemath-symbolics` is not present, then `is_Expression` should just return `False` for every input.
-
-Possible implementation strategies:
-- Introduce stub classes, make the actual implementation classes subclasses. For example, define class `Expression_base`; make `Expression` a subclass; `is_Expression(x)` tests `isinstance(x, Expression_base`
-  - `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
-  - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)
-
-- `try: import ... except ImportError: return False`
+The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.

+We propose to create stub classes in `sage.structure.element`, `sage.structure.parent` or in similar basic required modules, and make the actual implementation classes subclasses. This is already done for a few existing classes:
+- `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
+- `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)
+
+We add the following new stub classes:
+- in `sage.structure.element`, a class `Expression`
+  - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
+  - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)
+
+- in `sage.structure.element`, a class `Polyhedron`
+  - `Polyhedron_base` will be its only subclass
+

mkoeppe commented 3 years ago

comment:6

Thanks for these thoughts and experiments. I also prefer the approach using new base classes; and also I have learned more about the multiyear effort to get rid of is_... methods. I have updated the ticket description accordingly

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -16,4 +16,4 @@

 - in `sage.structure.element`, a class `Polyhedron`
   - `Polyhedron_base` will be its only subclass
-
+  - `Polyhedron.__new__` delegates to what is now `sage.geometry.polyhedron.constructor.Polyhedron`

mkoeppe commented 3 years ago

Branch: u/mkoeppe/replace_some_is_____functions_by_isinstance_with_new_base_classes_to_enable_modularization

mkoeppe commented 3 years ago

comment:10

Here's an attempt for Polyhedron. Still need to do some trick to make the constructor's docstring available

New commits:

`4f0345b`	`sage.structure.element.Polyhedron: New base class / constructor`
`f7f15a7`	`is_Polyhedron: Deprecate`

mkoeppe commented 3 years ago

Commit: f7f15a7

mkoeppe commented 3 years ago

Author: Matthias Koeppe, ...

mkoeppe commented 3 years ago

comment:12

For getting the docstring I have tried to make __doc__ a property, but this approach does not seem to work for Cython classes...

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from f7f15a7 to 9552be9

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

9552be9 WIP: Try to make `__doc__` a class property

mkoeppe commented 3 years ago

comment:14

I think I'll need some help from Cython experts here...

tscrim commented 3 years ago

comment:15

I think you're running into the fact that the Python special double underscore methods are handled in a special way. I don't know too much about how that's done, but that that is my recollection.

mkoeppe commented 3 years ago

comment:16

A different approach would be to keep the base class / constructor in sage.geometry.polyhedron.constructor (similar to the branch of #26366) and to assign this module to the distribution sagemath-categories.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -10,10 +10,10 @@
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 We add the following new stub classes:
-- in `sage.structure.element`, a class `Expression`
+- in `sage.structure.element` (or `sage.symbolic.constructor`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

-- in `sage.structure.element`, a class `Polyhedron`
+- in `sage.structure.element` (or `sage.geometry.polyhedron.constructor`?), a class `Polyhedron`
   - `Polyhedron_base` will be its only subclass
   - `Polyhedron.__new__` delegates to what is now `sage.geometry.polyhedron.constructor.Polyhedron`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -10,10 +10,13 @@
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 We add the following new stub classes:
-- in `sage.structure.element` (or `sage.symbolic.constructor`?), a class `Expression`
+- in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

-- in `sage.structure.element` (or `sage.geometry.polyhedron.constructor`?), a class `Polyhedron`
-  - `Polyhedron_base` will be its only subclass
-  - `Polyhedron.__new__` delegates to what is now `sage.geometry.polyhedron.constructor.Polyhedron`
+- in `sage.geometry.polyhedron.abc`, a class `Polyhedron`
+  - `Polyhedron_base` will be its only direct subclass
+
+- #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.
+
+

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -5,7 +5,7 @@
 The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.

-We propose to create stub classes in `sage.structure.element`, `sage.structure.parent` or in similar basic required modules, and make the actual implementation classes subclasses. This is already done for a few existing classes:
+We propose to create stub classes in `sage.structure.element`, `sage.structure.parent`, and/or `sage.PAC.KAGE.abc`, and make the actual implementation classes subclasses (mostly, unique direct subclasses). This is already done for a few existing classes:
 - `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

mkoeppe commented 3 years ago

Changed commit from 9552be9 to none

mkoeppe commented 3 years ago

Changed author from Matthias Koeppe, ... to none

mkoeppe commented 3 years ago

Changed branch from u/mkoeppe/replace_some_is_____functions_by_isinstance_with_new_base_classes_to_enable_modularization to none

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,15 +1,16 @@
 `git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.
-
 This pattern is an obstacle to modularization.

-The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard.
+The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard:
+The module providing the class must be installed so that the import does not fail.

+This can of course be worked around by `try... except` around `import`, as done for example in #32455, but we should avoid cluttering the code with this type of workaround.

-We propose to create stub classes in `sage.structure.element`, `sage.structure.parent`, and/or `sage.PAC.KAGE.abc`, and make the actual implementation classes subclasses (mostly, unique direct subclasses). This is already done for a few existing classes:
+Instead we propose to create stub classes (abstract base classes) in `sage.structure.element`, `sage.structure.parent`, and/or `sage.PAC.KAGE.abc`, and make the actual implementation classes subclasses (mostly, unique direct subclasses). This pattern already exists in the Sage code for a few existing classes:
 - `sage.structure.element` defines a base class `Matrix` (for purposes of the coercion system) and a function `is_Matrix` (see #32347)
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

-We add the following new stub classes:
+We add the following new classes:
 - in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)
@@ -19,4 +20,7 @@

 - #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

+Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

+As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.
+

mkoeppe commented 3 years ago

comment:21

Revised the description - no longer attempt for the abc and the constructor to be the same object. This removes all of the technical difficulties discussed above.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-`git grep 'import.* is_[A-Z]'` reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.
+`git grep 'import.* is_[A-Z]'` (or `git grep 'import.*is_[A-Z]' | grep -E -v 'is_(Ring|Matrix)[^A-Z]'  | grep -v 'sage:'`) reveals a common pattern in our code: We import a module just to check whether a user-provided object belongs to a class defined by that module.
 This pattern is an obstacle to modularization.

 The ongoing effort (#12824, https://groups.google.com/g/sage-devel/c/wEzb0awmyN0, #13370, #14329, #15076, #15098, #15333, #18173, #24443, #24525, #27593, #28470, #32347) to replace this pattern by importing the class and `isinstance` does not help in this regard:

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -11,15 +11,15 @@
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 We add the following new classes:
-- in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
+- #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.
+
+- #32599: in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

 - in `sage.geometry.polyhedron.abc`, a class `Polyhedron`
   - `Polyhedron_base` will be its only direct subclass

-- #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.
-
 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

 As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -13,6 +13,8 @@
 We add the following new classes:
 - #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

+- #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`
+
 - #32599: in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -15,7 +15,7 @@

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

-- #32599: in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
+- in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -11,7 +11,7 @@
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 We add the following new classes:
-- #32566: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.
+- #32566/#32610: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -11,7 +11,7 @@
 - `sage.structure.parent` defines a base class `Set_generic`; using it with `isinstance` has replaced use of the earlier `is_Set` function (#24443, #28470)

 We add the following new classes:
-- #32566/#32610: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.
+- #32566/#32610/#32612: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -19,7 +19,7 @@
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

-- in `sage.geometry.polyhedron.abc`, a class `Polyhedron`
+- #32637: in `sage.geometry.abc`, a class `Polyhedron`
   - `Polyhedron_base` will be its only direct subclass

 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -15,7 +15,7 @@

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

-- in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
+- #32638: in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -15,8 +15,8 @@

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

-- #32638: in `sage.structure.element` (or `sage.symbolic.abc`?), a class `Expression`
-  - the existing class `sage.symbolic.expression.Expression` will be renamed to `Expression_pynac` and be its only subclass
+- #32638: in `sage.structure.element`, a class `Expression`
+  - the existing class `sage.symbolic.expression.Expression` will be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

 - #32637: in `sage.geometry.abc`, a class `Polyhedron`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -15,6 +15,10 @@

 - #32606: Replace `is_IntegerModRing` by `isinstance` with new class `sage.rings.abc.IntegerModRing`

+- #32660: Add `sage.rings.abc.AlgebraicField` etc., deprecate `is_AlgebraicField`
+
+- #32664: Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
+
 - #32638: in `sage.structure.element`, a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -19,7 +19,7 @@

 - #32664: Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`

-- #32638: in `sage.structure.element`, a class `Expression`
+- #32638/#32665: in `sage.structure.element`, a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -26,6 +26,8 @@
 - #32637: in `sage.geometry.abc`, a class `Polyhedron`
   - `Polyhedron_base` will be its only direct subclass

+- #32709: `sage.structure.element`: Add ABCs `Polynomial`, `MPolynomial` for `isinstance` testing
+
 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

 As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -30,5 +30,6 @@

 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

-As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.
+As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** #32637 adds doctests to document and enforce this design. Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

+

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -32,4 +32,7 @@

 As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** #32637 adds doctests to document and enforce this design. Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

+Tickets not needed for modularization, but removing `is_` functions in favor of `isinstance` for uniformity of the codebase:
+- #32664 Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
+- #32738 Deprecate `is_Algebra`, `is_FreeAlgebra`, `is_QuaternionAlgebra`, `is_SymmetricFunctionAlgebra`

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -17,8 +17,6 @@

 - #32660: Add `sage.rings.abc.AlgebraicField` etc., deprecate `is_AlgebraicField`

-- #32664: Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
-
 - #32638/#32665: in `sage.structure.element`, a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -17,6 +17,8 @@

 - #32660: Add `sage.rings.abc.AlgebraicField` etc., deprecate `is_AlgebraicField`

+- #32750: `sage.rings.abc`: Add `pAdicRing`, `pAdicField`; deprecate `is_pAdic...`
+
 - #32638/#32665: in `sage.structure.element`, a class `Expression`
   - the existing class `sage.symbolic.expression.Expression` will be its only subclass
   - deprecate `is_Expression`, `is_SymbolicEquation`, `is_CallableSymbolicExpression`, `is_SymbolicVariable`; replace uses by `isinstance(x, Expression)` and a method call (may need a new method `is_callable`)

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -35,4 +35,5 @@
 Tickets not needed for modularization, but removing `is_` functions in favor of `isinstance` for uniformity of the codebase:
 - #32664 Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
 - #32738 Deprecate `is_Algebra`, `is_FreeAlgebra`, `is_QuaternionAlgebra`, `is_SymmetricFunctionAlgebra`
+- #34307 Deprecate `is_Polyhedron`, `is_Cone`, `is_LatticePolytope`

mkoeppe commented 1 year ago

Description changed:

--- 
+++ 
@@ -36,4 +36,6 @@
 - #32664 Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
 - #32738 Deprecate `is_Algebra`, `is_FreeAlgebra`, `is_QuaternionAlgebra`, `is_SymmetricFunctionAlgebra`
 - #34307 Deprecate `is_Polyhedron`, `is_Cone`, `is_LatticePolytope`
+- #34804 Deprecate `sage.interfaces` `is_...Element` functions

+

mkoeppe commented 1 year ago

Description changed:

--- 
+++ 
@@ -28,14 +28,17 @@

 - #32709: `sage.structure.element`: Add ABCs `Polynomial`, `MPolynomial` for `isinstance` testing

+- #34804 Deprecate `sage.interfaces` `is_...Element` functions
+
+- #32664 Deprecate `is_FiniteField`, make `sage.rings.finite_rings` a namespace package
+
+
 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

 As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** #32637 adds doctests to document and enforce this design. Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

 Tickets not needed for modularization, but removing `is_` functions in favor of `isinstance` for uniformity of the codebase:
-- #32664 Add `sage.rings.abc.FiniteField`, deprecate `is_FiniteField`
 - #32738 Deprecate `is_Algebra`, `is_FreeAlgebra`, `is_QuaternionAlgebra`, `is_SymmetricFunctionAlgebra`
 - #34307 Deprecate `is_Polyhedron`, `is_Cone`, `is_LatticePolytope`
-- #34804 Deprecate `sage.interfaces` `is_...Element` functions

mkoeppe commented 1 year ago

Description changed:

--- 
+++ 
@@ -26,7 +26,7 @@
 - #32637: in `sage.geometry.abc`, a class `Polyhedron`
   - `Polyhedron_base` will be its only direct subclass

-- #32709: `sage.structure.element`: Add ABCs `Polynomial`, `MPolynomial` for `isinstance` testing
+- #32709: Add ABCs `CommutativePolynomial`, `MPolynomial_libsingular`, `InfinitePolynomial`; deprecate `is_Polynomial`, `is_MPolynomial`

 - #34804 Deprecate `sage.interfaces` `is_...Element` functions

mkoeppe commented 1 year ago

Description changed:

--- 
+++ 
@@ -32,6 +32,7 @@

 - #32664 Deprecate `is_FiniteField`, make `sage.rings.finite_rings` a namespace package

+- #34931 New ABC `sage.structure.element.NumberFieldElement`, deprecate `is_NumberFieldElement`

 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

mkoeppe commented 1 year ago

Description changed:

--- 
+++ 
@@ -34,6 +34,8 @@

 - #34931 New ABC `sage.structure.element.NumberFieldElement`, deprecate `is_NumberFieldElement`

+- #24525 deprecate `is_RealNumber`, `is_AlgebraicNumber`, `is_RealIntervalFieldElement`, ...
+
 Technical limitation: If a `sage.PAC.KAGE.abc` module is intended to provide the new classes, then `sage.PAC.KAGE` must be prepared to become a native namespace package. (See Meta-ticket #32501: Clear out `__init__.py` files in preparation for namespace packages)

 As of this ticket, **each of the new abstract base classes is intended to have a unique direct implementation subclass.** #32637 adds doctests to document and enforce this design. Attempting to define new hierarchies of abstract base classes is beyond the scope of this ticket. Also, the new classes do not define any methods.

sagemath / sage

Meta-ticket: Replace `is_Class` functions, create abstract base classes to enable modularization #32414

32566/#32610/#32612: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

32606

32660

32750

32638/#32665: in `sage.structure.element`, a class `Expression`

32637: in `sage.geometry.abc`, a class `Polyhedron`

32709

34804

32664

34931

24525

32738

34307

37895

37896

37897

37898

37922

37924

38022

38032

38035

38077

38103

38124

38125

38126

38128

38184

38266

38287

38297

38228

38240

29705

sagemath / sage

Meta-ticket: Replace `is_Class` functions, create abstract base classes to enable modularization #32414

32566/#32610/#32612: sage.rings.abc defines base class RealDoubleField for sage.rings.real_double.RealDoubleField_class etc.

32606

32660

32750

32638/#32665: in sage.structure.element, a class Expression

32637: in sage.geometry.abc, a class Polyhedron

32709

34804

32664

34931

24525

32738

34307

37895

37896

37897

37898

37922

37924

38022

38032

38035

38077

38103

38124

38125

38126

38128

38184

38266

38287

38297

38228

38240

29705

32566/#32610/#32612: `sage.rings.abc` defines base class `RealDoubleField` for `sage.rings.real_double.RealDoubleField_class` etc.

32638/#32665: in `sage.structure.element`, a class `Expression`

32637: in `sage.geometry.abc`, a class `Polyhedron`