#25686 closed defect (fixed)

UniversalCyclotomicField is not finite

Reported by: soehms Owned by:
Priority: major Milestone: sage-8.3
Component: algebra Keywords: days94
Cc: tscrim Merged in:
Authors: Sebastian Oehms Reviewers: Tomer Bauer, Luis Felipe Tabera
Report Upstream: N/A Work issues:
Branch: 03055ab (Commits) Commit: 03055ab738fc14f50e86a18ff42e8f5a5adc382c
Dependencies: Stopgaps:

Description

sage: UCF = UniversalCyclotomicField()
sage: UCF.is_finite()
True

This is explicitly given in the code of universal_cyclotomic_field.py:

    def is_finite(self):
        r"""
        Returns ``True``.

        EXAMPLES::

            sage: UniversalCyclotomicField().is_finite()
            True

        .. TODO::

            this method should be provided by the category.
        """
        return True

This has been correct before a change in April 2015 (Trac #1852). It looks like a mistake in connection with that ticket. The old code from git looked like this:

author  Jean-Philippe Labbé <labbe@math.fu-berlin.de>   2015-04-29 17:56:59 +0300
committer       Jean-Philippe Labbé <labbe@math.fu-berlin.de>   2015-04-29 17:56:59 +0300
commit  dc3de84809792bde5e2cde8c8c579f42496e0647 (patch)
tree    2a5ed5f0daf452364580856388e5717cf47f1b60
parent  Updated Sage version to 6.7.beta3 (diff)
parent  Trac 18152: __neg__ for UCF elements (diff)


-    def is_finite(self):
-        r"""
-        Returns False as ``self`` is not finite.
-
-        EXAMPLES::
-
-            sage: UCF = UniversalCyclotomicField()
-            sage: UCF.is_finite()
-            False
-        """
-        return False

Thus the task might be to restore the old code!

Change History (17)

comment:1 Changed 17 months ago by tscrim

  • Cc tscrim added

comment:2 Changed 17 months ago by soehms

  • Branch set to u/soehms/ucf_not_finite

comment:3 Changed 17 months ago by soehms

  • Commit set to a5673d143b2368b3a21a9e7e6fa21106170778ff

We (Travis and me) tried to move the is_finite method to the category framework. But this lead into difficulties. Thus, we let it fall back to is_finite in rings.pyx replacing the raise of NotImplementedError to a super(Ring, self).is_finite()-call.


New commits:

a5673d1remove meth is_finite from ring.pyx and universal_cyclotomic_field in order to fall back to the categorial framework

comment:4 Changed 17 months ago by git

  • Commit changed from a5673d143b2368b3a21a9e7e6fa21106170778ff to 16b48b595e5a2159ba60d7015996c9d8075ab828

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

16b48b5correction

comment:5 Changed 17 months ago by git

  • Commit changed from 16b48b595e5a2159ba60d7015996c9d8075ab828 to ae0cbb88c251edf2a7380f687fd5aae7d2a79760

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

ae0cbb8correction of comment

comment:6 Changed 17 months ago by soehms

  • Status changed from new to needs_review

New commits:

ae0cbb8correction of comment

comment:7 Changed 17 months ago by lftabera

You should add a test that shows that the bug is fixed. Since you have deleted the method, this test has to be added elsewhere. In this case, it can be added to the general set of tests at the beggining of the universal_cyclotomic_field.py file

comment:8 Changed 17 months ago by git

  • Commit changed from ae0cbb88c251edf2a7380f687fd5aae7d2a79760 to d3b2c00f2ef0f22389d3b30890d322479f6aa902

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

d3b2c00test added

comment:9 Changed 17 months ago by mathzeta2

  • Reviewers set to Tomer Bauer
  • Status changed from needs_review to positive_review

comment:10 Changed 17 months ago by lftabera

  • Reviewers changed from Tomer Bauer to Tomer Bauer, Luis Felipe Tabera

comment:11 Changed 17 months ago by vbraun

  • Status changed from positive_review to needs_work

See patchbot

comment:12 Changed 17 months ago by mathzeta2

The line return super(Ring, self).is_finite() is probably the culprit, we need to update the error message of that test.

Last edited 17 months ago by mathzeta2 (previous) (diff)

comment:13 Changed 17 months ago by git

  • Commit changed from d3b2c00f2ef0f22389d3b30890d322479f6aa902 to 03055ab738fc14f50e86a18ff42e8f5a5adc382c

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

03055abdoctest error in _list_from_iterator fixed

comment:14 Changed 17 months ago by soehms

  • Status changed from needs_work to needs_review

comment:15 Changed 17 months ago by mathzeta2

Works for me. If the patchbot is happy, then positive review.

comment:16 Changed 17 months ago by soehms

  • Status changed from needs_review to positive_review

comment:17 Changed 17 months ago by vbraun

  • Branch changed from u/soehms/ucf_not_finite to 03055ab738fc14f50e86a18ff42e8f5a5adc382c
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.