sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

Properly initialize symbolic rings #21893

Closed saraedum closed 7 years ago

saraedum commented 7 years ago

Currently

sage: f(x) = 1
sage: TestSuite(f.parent()).run()
AttributeError
sage: 'sum' in dict(f.parent())
True
sage: f.parent().sum
AttributeError

This is because this symbolic ring does not call its super classes __init__ and therefore it does not inherit correctly the methods of its category.

Also, some tests do not pass because the pickling provided by factory is not properly used:

sage: f.parent()._test_pickling()
AssertionError

Component: symbolics

Author: Julian Rüth

Branch/Commit: 36c6bb0

Reviewer: David Roe

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

saraedum commented 7 years ago

Branch: u/saraedum/properly_initialize_symbolic_rings

saraedum commented 7 years ago

Description changed:

--- 
+++ 
@@ -4,9 +4,14 @@
 sage: f(x) = 1
 sage: TestSuite(f.parent()).run()
 AttributeError
+sage: 'sum' in dict(f.parent())
+True
+sage: f.parent().sum
+AttributeError

+This is because this symbolic ring does not call its super classes __init__ and therefore it does not inherit correctly the methods of its category.

-Also, some tests do not pass because the factory is not properly used: +Also, some tests do not pass because the pickling provided by factory is not properly used:

 sage: f.parent()._test_pickling()
saraedum commented 7 years ago

New commits:

a770337SymbolicRing is a singleton
901bae3SymbolicRing is a singleton
5979aefCallableSymbolicExpressionRing_class get only created through a factory
5d32dbdProperly call the super class constructors of CallableSymbolicExpressionRing_class
36c6bb0Run test suites for symbolic rings
saraedum commented 7 years ago

Commit: 36c6bb0

saraedum commented 7 years ago
comment:3

The tests in symbolic/ pass. Let's see what the patchbot thinks.

The commits should make sense one by one. The reviewer might want to review them individually.

roed314 commented 7 years ago
comment:4

Looks good once patchbot approves.

roed314 commented 7 years ago

Reviewer: David Roe

vbraun commented 7 years ago

Changed branch from u/saraedum/properly_initialize_symbolic_rings to 36c6bb0