Opened 10 years ago
Closed 10 years ago
#12988 closed enhancement (fixed)
characteristic() should be an integer
Reported by: | saraedum | Owned by: | AlexGhitza |
---|---|---|---|
Priority: | trivial | Milestone: | sage-5.1 |
Component: | algebra | Keywords: | ZZ characteristic sd40.5 |
Cc: | Merged in: | sage-5.1.beta2 | |
Authors: | Julian Rueth | Reviewers: | David Roe, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #13043 | Stopgaps: |
Description
Currently characteristic()
does not return an Integer
for some rings:
sage: type(ZZ.characteristic()) <type 'int'>
This is annoying because one cannot for example do:
sage: ZZ.characteristic().is_prime() AttributeError: 'int' object has no attribute 'is_prime'
I don't see a good reason not to make characteristic always an Integer. The minimal performance loss should not be an issue in any computation that I can imagine.
Attachments (2)
Change History (14)
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Keywords sd40.5 added
comment:3 Changed 10 years ago by
- Status changed from needs_review to needs_work
It seems like the test suite is not being run:
drake@sagenb:~/s/sage-5.1.beta0$ ./sage -t devel/sage/sage/categories/algebras_with_basis.py sage -t "devel/sage/sage/categories/algebras_with_basis.py" ********************************************************************** File "/home/drake/s/sage-5.1.beta0/devel/sage/sage/categories/algebras_with_basis.py", line 60: sage: TestSuite(A).run(verbose=True) Expected: running ._test_additive_associativity() . . . pass running ._test_an_element() . . . pass running ._test_associativity() . . . pass running ._test_category() . . . pass running ._test_characteristic() . . . pass running ._test_distributivity() . . . pass running ._test_elements() . . . Running the test suite of self.an_element() running ._test_category() . . . pass running ._test_eq() . . . pass running ._test_not_implemented_methods() . . . pass running ._test_pickling() . . . pass pass running ._test_elements_eq() . . . pass running ._test_eq() . . . pass running ._test_not_implemented_methods() . . . pass running ._test_one() . . . pass running ._test_pickling() . . . pass running ._test_prod() . . . pass running ._test_some_elements() . . . pass running ._test_zero() . . . pass Got: running ._test_additive_associativity() . . . pass running ._test_an_element() . . . pass running ._test_associativity() . . . pass running ._test_category() . . . pass running ._test_distributivity() . . . pass running ._test_elements() . . . Running the test suite of self.an_element() running ._test_category() . . . pass running ._test_eq() . . . pass running ._test_not_implemented_methods() . . . pass running ._test_pickling() . . . pass pass running ._test_elements_eq() . . . pass running ._test_eq() . . . pass running ._test_not_implemented_methods() . . . pass running ._test_one() . . . pass running ._test_pickling() . . . pass running ._test_prod() . . . pass running ._test_some_elements() . . . pass running ._test_zero() . . . pass
It's expecting to see the characteristic test, but that test is not run. I don't know the categories and test frameworks well enough to fix this. Do you see what's wrong?
comment:4 Changed 10 years ago by
- Dependencies set to #13043
That's strange. I'll run the doctests again to see if this also happens here.
Actually _test_characteristic
didn't catch wrong characteristic()
implementations in some cases which is related to #13043. I'll upload an updated patch soon.
comment:5 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:6 Changed 10 years ago by
This now passes doctests, and every example that I've tried successfully returns Integer characteristic. But I would like someone who understands the category framework and the test framework a bit better to look at this. Consider your patch to have a weak positive review.
comment:7 Changed 10 years ago by
- Status changed from needs_review to positive_review
This looks good to me.
comment:8 Changed 10 years ago by
- Reviewers set to David Roe
comment:9 Changed 10 years ago by
- Status changed from positive_review to needs_work
3 doctest failures with sage-5.1.beta1:
sage -t "devel/sage-main/sage/rings/padics/padic_base_leaves.py" ********************************************************************** File "/usr/local/src/sage-5.1.beta1/devel/sage-main/sage/rings/padics/padic_base_leaves.py", line 168: sage: TestSuite(R).run() Expected nothing Got: Failure in _test_characteristic: Traceback (most recent call last): File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/misc/sage_unittest.py", line 275, in run test_method(tester = tester) File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/categories/rings.py", line 258, in _test_characteristic tester.assertEqual(type(characteristic),sage.rings.integer.Integer) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 509, in assertEqual assertion_func(first, second, msg=msg) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 502, in _baseAssertEqual raise self.failureException(msg) AssertionError: <type 'int'> != <type 'sage.rings.integer.Integer'> ------------------------------------------------------------ The following tests failed: _test_characteristic ********************************************************************** File "/usr/local/src/sage-5.1.beta1/devel/sage-main/sage/rings/padics/padic_base_leaves.py", line 175: sage: TestSuite(R).run() Expected nothing Got: Failure in _test_characteristic: Traceback (most recent call last): File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/misc/sage_unittest.py", line 275, in run test_method(tester = tester) File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/categories/rings.py", line 258, in _test_characteristic tester.assertEqual(type(characteristic),sage.rings.integer.Integer) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 509, in assertEqual assertion_func(first, second, msg=msg) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 502, in _baseAssertEqual raise self.failureException(msg) AssertionError: <type 'int'> != <type 'sage.rings.integer.Integer'> ------------------------------------------------------------ The following tests failed: _test_characteristic ********************************************************************** File "/usr/local/src/sage-5.1.beta1/devel/sage-main/sage/rings/padics/padic_base_leaves.py", line 182: sage: TestSuite(R).run() Expected nothing Got: Failure in _test_characteristic: Traceback (most recent call last): File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/misc/sage_unittest.py", line 275, in run test_method(tester = tester) File "/usr/local/src/sage-5.1.beta1/local/lib/python/site-packages/sage/categories/rings.py", line 258, in _test_characteristic tester.assertEqual(type(characteristic),sage.rings.integer.Integer) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 509, in assertEqual assertion_func(first, second, msg=msg) File "/usr/local/src/sage-5.1.beta1/local/lib/python2.7/unittest/case.py", line 502, in _baseAssertEqual raise self.failureException(msg) AssertionError: <type 'int'> != <type 'sage.rings.integer.Integer'> ------------------------------------------------------------ The following tests failed: _test_characteristic **********************************************************************
comment:10 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:11 Changed 10 years ago by
- Reviewers changed from David Roe to David Roe, Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:12 Changed 10 years ago by
- Merged in set to sage-5.1.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
I believe it is easiest to check that this is correct through the test framework of the categories.
I added a test to Rings and also one to Fields() checking that it's a prime or zero.