Fix TestSuite failures for schemes

[nthiery]

Consider the following situation:

sage: S = Spec(ZZ)
sage: x = S.an_element()

Running TestSuite(S) and TestSuite(x) yields several failures. A related problem is

sage: S
Spectrum of Integer Ring
sage: parent(x)
Set of rational points of Spectrum of Integer Ring

whereas we expect parent(x) is S.

Here is the complete TestSuite report:

sage: TestSuite(S).run()
Failure in _test_an_element:
Traceback (most recent call last):
...
NotImplementedError
------------------------------------------------------------
  Failure in _test_category:
  Traceback (most recent call last):
  ...
  AssertionError: False is not true
  ------------------------------------------------------------
  Failure in _test_pickling:
  Traceback (most recent call last):
  ...
  AssertionError: Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring != Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring
  ------------------------------------------------------------
  The following tests failed: _test_category, _test_pickling
Failure in _test_elements
Failure in _test_some_elements:
Traceback (most recent call last):
...
NotImplementedError
------------------------------------------------------------
The following tests failed: _test_an_element, _test_elements, _test_some_elements

sage: TestSuite(x).run()
Failure in _test_category:
Traceback (most recent call last):
...
AssertionError: False is not true
------------------------------------------------------------
Failure in _test_pickling:
Traceback (most recent call last):
...
AssertionError: Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring != Point on Spectrum of Integer Ring defined by the Principal ideal (991) of Integer Ring
------------------------------------------------------------
The following tests failed: _test_category, _test_pickling

(Note: the NotImplementedError that one gets after applying #16158 is a different one than before.)

Depends on #16158

CC: @vbraun

Component: algebraic geometry

Author: Peter Bruin

Branch/Commit: 5fd92ee

Reviewer: Travis Scrimshaw

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

[novoselt]

comment:3

#11599 claims to fix this, but I am not entirely sure what would it take to fix this ticket - the reported category is now schemes and only 3 of the TestSuite tests fail.

[pjbruin]

comment:6

What is Spec intended to mean in the first place? The definition currently starts with

class Spec(AffineScheme):
    ....

This suggests that Spec is a special kind of affine scheme. But aren't "the spectrum of a ring" and "an affine scheme" exactly the same thing?

This code probably predates the category framework. A more modern design would perhaps be something like the following:

• define the category AffineSchemes (or Schemes().Affine()) of affine schemes;
• designate AffineScheme as the "canonical" type for objects in this category, and move functionality from Spec to AffineScheme as appropriate;
• convert Spec into a functor from CommutativeRings to AffineSchemes.
  (Mathematically speaking, Spec [as a functor from commutative rings to schemes, or even locally ringed spaces] can be defined as the adjoint functor of the global sections functor X -> O_X(X) from schemes to commutative rings, and this gives an anti-equivalence of categories from commutative rings to affine schemes; I wonder if this could somehow be formalised in Sage's category framework, but that's another story...)

[Edit: the above idea is now essentially #16158, although there is no category of affine schemes yet.]

[pjbruin]

Dependencies: #16158

[pjbruin]

comment:7

To show that the first part of the original ticket is fixed:

sage: S = Spec(ZZ)
sage: S.category()
Category of Schemes
sage: isinstance(S, S.category().parent_class) 
True

[pjbruin]

Branch: u/pbruin/7946-scheme_point_improvements

[pjbruin]

comment:8

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

[pjbruin]

Author: Peter Bruin

[pjbruin]

Commit: 5fd92ee

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:11

Replying to @pjbruin:

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

Is there some way you can get rid of that custom __call__ method in AffineScheme, or at least passes things up through the Parent.__call__ when trying to construct an element? If not, then I think we're okay setting this to positive review as it fixes the issues as stated in the ticket description (you can do this on my behalf).

Also, I think that comment:6 (from the very limited understanding I have of the math) is the correct way to do things. However that would be a major overhaul to be done on a separate ticket, and perhaps the above change to __call__ would fit into that.

[pjbruin]

comment:12

Replying to @tscrim:

Replying to @pjbruin:

The attached branch makes affine schemes, and points on them, cooperate better with the category and coercion code. The TestSuite now runs without failures.

Is there some way you can get rid of that custom __call__ method in AffineScheme, or at least passes things up through the Parent.__call__ when trying to construct an element? If not, then I think we're okay setting this to positive review as it fixes the issues as stated in the ticket description (you can do this on my behalf).

The reason this is done via the __call__() method is that an affine scheme S accepts two kinds of input; only one of them (input is a prime ideal) constructs an element with S as its parent. In the other case (input is a list of coordinates) the __call__() syntax constructs an element of a suitable scheme homset. I don't see an easy way to avoid a custom __call__() method; it may be possible (and would be nice) to get rid of it, but it is better to do this on separate ticket. (Note also that the __call__() method was already present, I just improved it as necessary.)

Also, I think that comment:6 (from the very limited understanding I have of the math) is the correct way to do things. However that would be a major overhaul to be done on a separate ticket, and perhaps the above change to __call__ would fit into that.

What I was talking about in that comment was mostly done on #16158 (closed a couple of months ago). I think the remaining issue of making a category AffineSchemes would probably be disjoint from getting rid of the __call__() method.

I'm setting this to positive review since you gave the green light.

[tscrim]

comment:13

The reason this is done via the __call__() method is that an affine scheme S accepts two kinds of input; only one of them (input is a prime ideal) constructs an element with S as its parent. In the other case (input is a list of coordinates) the __call__() syntax constructs an element of a suitable scheme homset. I don't see an easy way to avoid a custom __call__() method; it may be possible (and would be nice) to get rid of it, but it is better to do this on separate ticket. (Note also that the __call__() method was already present, I just improved it as necessary.)

That's what I was thinking from a quick look through. Anyways, for later.

What I was talking about in that comment was mostly done on #16158 (closed a couple of months ago). I think the remaining issue of making a category AffineSchemes would probably be disjoint from getting rid of the __call__() method.

I knew the last one was done, but forgot that it moved the functionality. Unfortunately I think you're right, but we can at least do a followup to create the category of AffineSchemes.

I'm setting this to positive review since you gave the green light.

Thanks.

[vbraun]

Changed branch from u/pbruin/7946-scheme_point_improvements to 5fd92ee