Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#11490 closed enhancement (fixed)

Add a thematic tutorial on coercion and categories

Reported by: SimonKing Owned by: mvngu
Priority: major Milestone: sage-5.8
Component: documentation Keywords: categories coercion thematic tutorial
Cc: novoselt, vbraun Merged in: sage-5.8.beta1
Authors: Simon King Reviewers: Vincent Delecroix, Travis Scrimshaw, Dmitrii Pasechnik
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14084 Stopgaps:

Description (last modified by SimonKing)

From time to time people state that a good introduction to the category framework and Sage's coercion system is missing (at sage-devel, for example).

I suggest to include a thematic tutorial.

Note that it may relate with #8821, which is about adding a section on coercion (but not categories) in the guided tour.

Attachments (2)

CoercionAndCategories.sws (20.8 KB) - added by SimonKing 8 years ago.
Worksheet on how to use category framework and coercion when implementing parents and elements
trac11490-coercion_tutorial.patch (68.7 KB) - added by SimonKing 7 years ago.
Add a thematic tutorial on categories and coercion

Download all attachments as: .zip

Change History (56)

comment:1 Changed 8 years ago by SimonKing

  • Authors set to Simon King
  • Description modified (diff)

I suggest to base a thematic tutorial on this worksheet.

It covers base classes for parents and elements, category framework for parents and elements, uniqueness of parents, coercion maps, construction functors, and test suites.

The didactical approach is to explain the theory and illustrate each theoretical bit by showing how it can be implemented. Step by step, one obtains an alternative implementation of fraction fields.

There are also several texts in the combinat branch, and I suggest to add these as well. The style of these texts is very different from the style of my worksheet, and I hope that the user will benefit from diversity.

comment:2 Changed 8 years ago by SimonKing

  • Status changed from new to needs_review

I attached my worksheet.

Changed 8 years ago by SimonKing

Worksheet on how to use category framework and coercion when implementing parents and elements

comment:3 Changed 8 years ago by SimonKing

I did some changes in my worksheet (updating both the published worksheet and the attached sws file). For some stupid reason, in the original version, I had fraction fields only for unique factorisation domains, not for integral domains.

comment:4 Changed 8 years ago by jhpalmieri

I've only just glanced at your worksheet, but how do its contents compare to the section on coercion in the reference manual?

comment:5 Changed 8 years ago by jason

  • Status changed from needs_review to needs_work

How is this needs review? There isn't a patch to apply. It certainly seems like good progress, but still is needs_work.

comment:6 Changed 8 years ago by SimonKing

  • Status changed from needs_work to needs_info

And what else should it be than a worksheet?

comment:7 Changed 8 years ago by jason

A thematic tutorial in the documentation is a rst file. See $SAGE_ROOT/devel/sage/doc/en/thematic_tutorials/ for examples. A worksheet is probably pretty straightforward to convert to a rst file.

So I would think the patch would be an rst file in that directory and a one-line addition to the index.rst file in that directory.

comment:8 Changed 7 years ago by SimonKing

  • Dependencies set to #14084
  • Keywords thematic tutorial added
  • Status changed from needs_info to needs_review

Using #10637 and some manual work, I have translated the worksheet into a thematic tutorial in .rst format.

One of the example needs #14084 to work, hence, this is a dependency.

The tutorial links to the reference manual, which is only possible with #6495; however, I don't think this should be considered a dependency, because in the worst case the links are not available.

Needs review! After applying the patch, do sage --docbuild thematic_tutorials html.

Apply trac11490-coercion_tutorial.patch

comment:9 follow-up: Changed 7 years ago by vdelecroix

Hello,

I was wondering for such a patch to come! I read it because I am interested but by far not an expert. Here are some comments for trac11490-coercion_tutorial.patch, line numbers refer to the patch.

  • you may write "and" or "respectively" instead of "resp." (line 34)
  • at lines 195-198 there is a kind of magic to test the pickling. You shoud add few words to explain what's happening.
  • I do not like the class MyFrac(MyFrac) at lines 462, 812, 882 (but do not have better option)
  • I would like to know why test at line 494 fails while I was able to add some elements at line 294
  • line 678: write "P1 == P2" instead of "P1==P2" (ie add whitespaces on both sides of "==")
  • I do not like your example at line 687 because of
    sage: P1 = QQ['v,w']; P2 = ZZ['w,v']
    sage: v1 = P1('v'); w1 = P1('w')
    sage: v2 = P2('v'); w2 = P2('w')
    sage: (v1 + w2).parent() is P1
    True
    sage: (v2 + w1).parent() is P1
    True
    
    In other words, P1 is prefered to P2 ! Why is the reason ? I expected that are natural coercions in both directions then the parent of elements a+b is determined by the parent of a. Actually, there is an explanation in the lines after...

Best, Vincent

comment:10 in reply to: ↑ 9 Changed 7 years ago by SimonKing

Replying to vdelecroix:

  • you may write "and" or "respectively" instead of "resp." (line 34)

OK.

  • at lines 195-198 there is a kind of magic to test the pickling. You shoud add few words to explain what's happening.

Well, in the lines in front of it, I write

    However, for
    making the example work in the Sage's doctesting framework, we need to
    declare our class as an attribute of the ``__main__`` module.

I thought this is an explanation? Technically, the doctest framework finds that the classes are defined in the module __main__, but apparently (for a reason that is not clear to me) it can not find it in __main__---although this perfectly works in an interactive session.

Similar case is around line 1433 and 1572.

  • I do not like the class MyFrac(MyFrac) at lines 462, 812, 882 (but do not have better option)

The didactic idea is that it is more easy to understand what is happening, when only small changes are made. Hence, I certainly would not like to repeatedly give a full definition of MyFrac, each time changing or adding a small detail, because the reader would then be more likely to lose track.

  • I would like to know why test at line 494 fails while I was able to add some elements at line 294

At this point, we can add elements (visible in line 294), but P.zero_element() does not work yet, because this tries to return P(0), but conversion is not available at this point.

Shall I add the explanation at line 494, or better at line 539, when I made the summation work?

  • line 678: write "P1 == P2" instead of "P1==P2" (ie add whitespaces on both sides of "==")

OK.

  • I do not like your example at line 687 because of
    sage: P1 = QQ['v,w']; P2 = ZZ['w,v']
    sage: v1 = P1('v'); w1 = P1('w')
    sage: v2 = P2('v'); w2 = P2('w')
    sage: (v1 + w2).parent() is P1
    True
    sage: (v2 + w1).parent() is P1
    True
    
    In other words, P1 is prefered to P2 ! Why is the reason ?

There is a coercion from P2 to P1, but not from P1 to P2 (note the different base rings).

I expected that are natural coercions in both directions then the parent of elements a+b is determined by the parent of a.

That's correct, but there is no bi-directional coercion here.

OK, I'll post an update a bit later.

comment:11 follow-ups: Changed 7 years ago by tscrim

I also have the following comments:

  • There's a typo on line 100: This base class provides a lot mor methods than a general parent:: (mor -> more).
  • On line 133: Declaring the base or base is easy -- should that be "the base of base ring"?
  • On block starting at line 141, I would set the _repr_ and like functions in code formatting (double back ticks)
  • In the __cmp__ block, it might be worth noting that if __cmp__ is not implemented, bad things can happen when calling cmp (ex. #14065)
  • The double colon on line 312, I believe it will not show up in the documentation since I believe the newline will be treated as a whitespace.
  • The block starting at line 319, the last sentence is It not even gives a wrong answer, but results in an error::. I believe there should be a "does".
  • The but they are *required* or *optional* methods sentence on lines 435-436 sounds strange to me. I would recommend "so they are either *optional* or *required* methods."
  • The .. note:: on line 580 should have a blank line following it
  • On line 655, I think it should end with a bang So, *don't be afraid of using categories!* (sorry couldn't resist the pun)
  • I believe the title Not any conversion is a coercion on line 696 should be "Not every conversion is a coercion"
  • For the 5 axioms starting on line 725, shouldn't everything align (the blocks are one space less than the first line) for proper formatting?
  • On line 793 there is a typo: We have seen above that some onversions into our (onversions -> conversions)
  • Note block on line 935 needs a newline
  • The .. warning:: block on line 1536 needs a newline

For the record, I was reading it from the patch file and didn't look at the compiled doc, so some of these might be invalid.

One last thing, this is more from my personal taste, but I think the note/warning blocks should be capitalized as .. NOTE:: and .. WARNING:: to be consistent with the dev guide.

I'm sorry this turned into a bit of a laundry list of things. Thank you for this nice tutorial!

Best,
Travis

comment:12 in reply to: ↑ 11 ; follow-up: Changed 7 years ago by SimonKing

Replying to tscrim:

  • In the __cmp__ block, it might be worth noting that if __cmp__ is not implemented, bad things can happen when calling cmp (ex. #14065)

Didn't know that!

  • The block starting at line 319, the last sentence is It not even gives a wrong answer, but results in an error::. I believe there should be a "does".

You mean "It does not even give a wrong answer, but results in an error"?

  • The .. note:: on line 580 should have a blank line following it

Really? But the html output looks fine in that location.

  • For the 5 axioms starting on line 725, shouldn't everything align (the blocks are one space less than the first line) for proper formatting?

Looks fine in html.

  • Note block on line 935 needs a newline
  • The .. warning:: block on line 1536 needs a newline

Looks fine in html

That said, if it is sphinx convention to have a blank line, then I'll insert one.

One last thing, this is more from my personal taste, but I think the note/warning blocks should be capitalized as .. NOTE:: and .. WARNING:: to be consistent with the dev guide.

On some ticket (don't remember which one), I recall that both look the same in the output, but I don't recall whether we want it capitalised or not. So, if you say the dev guide recommends capitalisation, then I'll change it.

I'm sorry this turned into a bit of a laundry list of things.

Thank you!

comment:13 in reply to: ↑ 12 Changed 7 years ago by tscrim

Replying to SimonKing:

  • The block starting at line 319, the last sentence is It not even gives a wrong answer, but results in an error::. I believe there should be a "does".

You mean "It does not even give a wrong answer, but results in an error"?

Yes...

That said, if it is sphinx convention to have a blank line, then I'll insert one.

I'm just following the conventions page and that the TESTS:: and EXAMPLES:: blocks didn't format properly (although it might be because they are special).

One last thing, this is more from my personal taste, but I think the note/warning blocks should be capitalized as .. NOTE:: and .. WARNING:: to be consistent with the dev guide.

On some ticket (don't remember which one), I recall that both look the same in the output, but I don't recall whether we want it capitalised or not. So, if you say the dev guide recommends capitalisation, then I'll change it.

You are correct in that the html output is the same. Thank you, sorry to be a bit of a nag on this.

Best,
Travis

comment:14 in reply to: ↑ 11 ; follow-up: Changed 7 years ago by SimonKing

Replying to tscrim:

  • On block starting at line 141, I would set the _repr_ and like functions in code formatting (double back ticks)

That doesn't work, because double back ticks show up verbosely if they are in strong typesetting. Alternatively, I could remove the strong typesetting, if you prefer.

comment:15 in reply to: ↑ 14 Changed 7 years ago by tscrim

Replying to SimonKing:

Replying to tscrim:

  • On block starting at line 141, I would set the _repr_ and like functions in code formatting (double back ticks)

That doesn't work, because double back ticks show up verbosely if they are in strong typesetting. Alternatively, I could remove the strong typesetting, if you prefer.

Ah, didn't know that. In that case it is fine as it is.

comment:16 in reply to: ↑ 11 ; follow-up: Changed 7 years ago by SimonKing

Replying to tscrim:

  • In the __cmp__ block, it might be worth noting that if __cmp__ is not implemented, bad things can happen when calling cmp (ex. #14065)

You don't give negative examples in #14065. Do you mean things like the following?

sage: class Foo(Element):
....:     def __init__(self, x, parent=None):
....:         self.x = x
....:     def _repr_(self):
....:         return "<%s>"%self.x
....:     
sage: a = Foo(1,parent=ZZ)
sage: b = Foo(2,parent=ZZ)
sage: cmp(a,b)
Traceback (most recent call last):
...
NotImplementedError: BUG: sort algorithm for elements of 'None' not implemented

comment:17 in reply to: ↑ 16 Changed 7 years ago by tscrim

Replying to SimonKing:

Replying to tscrim:

  • In the __cmp__ block, it might be worth noting that if __cmp__ is not implemented, bad things can happen when calling cmp (ex. #14065)

You don't give negative examples in #14065. Do you mean things like the following?

sage: class Foo(Element):
....:     def __init__(self, x, parent=None):
....:         self.x = x
....:     def _repr_(self):
....:         return "<%s>"%self.x
....:     
sage: a = Foo(1,parent=ZZ)
sage: b = Foo(2,parent=ZZ)
sage: cmp(a,b)
Traceback (most recent call last):
...
NotImplementedError: BUG: sort algorithm for elements of 'None' not implemented

Yes, in addition if you implement __lt__ and like operators, it still breaks (for a easy version, inherit from CombinatorialObject, but then remove the Element inheritance and everything works).

Last edited 7 years ago by tscrim (previous) (diff)

comment:18 Changed 7 years ago by SimonKing

The patch is updated. I hope I have addressed all your remarks!

comment:19 follow-up: Changed 7 years ago by tscrim

If Vincent is happy with the changes with respect to his comments, I'll go back through and do the final review. Thanks.

comment:20 in reply to: ↑ 19 Changed 7 years ago by vdelecroix

Replying to tscrim:

If Vincent is happy with the changes with respect to his comments, I'll go back through and do the final review. Thanks.

Definitely happy ;-)

comment:21 follow-up: Changed 7 years ago by tscrim

One last little thing, the arrow point (for lack of a better name) on line 1107 is indented because of the space. Is this intended? It also is out of sync with the formatting of the arrow point on on line 436.

Thanks,
Travis

comment:22 Changed 7 years ago by novoselt

  • Cc novoselt added

comment:23 in reply to: ↑ 21 Changed 7 years ago by SimonKing

Replying to tscrim:

One last little thing, the arrow point (for lack of a better name) on line 1107 is indented because of the space. Is this intended? It also is out of sync with the formatting of the arrow point on on line 436.

I updated the patch, suggesting to replace the "arrow points" by a .. NOTE::.

comment:24 Changed 7 years ago by tscrim

  • Reviewers set to Vincent Delecroix, Travis Scrimshaw
  • Status changed from needs_review to positive_review

Looks good to me now too. Thank you for this nice tutorial.

comment:25 follow-up: Changed 7 years ago by dimpase

We want to implement a field

got me lost. Would you mind to be more specific? Are you talking about a particular field? Are you talking about the class of all fields? Something else?

comment:26 Changed 7 years ago by dimpase

IMHO it would be good if this was reviewed by someone without much clue about this stuff (like myself :)). It's a tutorial, after all, not a reference manual...

comment:27 in reply to: ↑ 25 Changed 7 years ago by SimonKing

Replying to dimpase:

We want to implement a field

got me lost. Would you mind to be more specific? Are you talking about a particular field? Are you talking about the class of all fields? Something else?

A few lines above, I write We illustrate the concepts of Sage’s category framework and coercion model by providing a toy implementation of fraction fields. Of course, I could repeat that statement.

comment:28 follow-up: Changed 7 years ago by SimonKing

I suggest to elaborate as follows:

The parent
----------

This tutorial explains Sage's category and coercion framework by means of a
detailled example, namely a toy implementation of fraction fields and fraction
field elements. Since we wish to implement a special kind of fields, it makes
sense to build on top of the base class :class:`sage.rings.ring.Field`
provided by Sage.

comment:29 Changed 7 years ago by SimonKing

  • Status changed from positive_review to needs_work

comment:30 Changed 7 years ago by SimonKing

  • Status changed from needs_work to needs_review

Since Dima wants to have a second look, I am removing the positive review for now.

comment:31 in reply to: ↑ 28 ; follow-up: Changed 7 years ago by dimpase

Replying to SimonKing:

I suggest to elaborate as follows:

The parent
----------

This tutorial explains Sage's category and coercion framework by means of a
detailled example, namely a toy implementation of fraction fields and fraction
field elements. Since we wish to implement a special kind of fields, it makes
sense to build on top of the base class :class:`sage.rings.ring.Field`
provided by Sage.

IMHO the 1st sentence,

This tutorial explains Sage's category and coercion framework by means of a
detailed example, namely a toy implementation of fraction fields and fraction
field elements.

belongs to the part above Outline. (and it's detailed with one 'l').

And then in "The parent" part you can say

Since we wish to implement a special kind of fields, namely, the fraction fields, it makes
sense to build on top of the base class :class:`sage.rings.ring.Field` provided by Sage.

(notice namely, the fraction fields that I added.)

comment:32 in reply to: ↑ 31 Changed 7 years ago by SimonKing

Replying to dimpase:

IMHO the 1st sentence,

This tutorial explains Sage's category and coercion framework by means of a
detailed example, namely a toy implementation of fraction fields and fraction
field elements.

belongs to the part above Outline.

Yes, but there already is a similar sentence above Outline! Do you suggest to replace it?

And then in "The parent" part you can say

Since we wish to implement a special kind of fields, namely, the fraction fields, it makes
sense to build on top of the base class :class:`sage.rings.ring.Field` provided by Sage.

(notice namely, the fraction fields that I added.)

OK.

comment:33 Changed 7 years ago by SimonKing

PS: I just think that actually we only have four axioms for coercion, not five. What I give as first axiom ("coercion is defined on all elements, while a conversion could be a partial map") is technically a consequence of the second axiom ("coercions are morphisms").

So, shall I combine the two statements?

comment:34 Changed 7 years ago by SimonKing

I have updated the patch according to what we have just discussed. If you like the old version better: I kept a backup...

comment:35 Changed 7 years ago by SimonKing

Sorry, I just made another change: Around line 1018, I ask: "How can we establish a coercion from P to \mathrm{Frac}(\ZZ[x])?"

But actually, in the end we do not establish such a coercion. Instead, we make Sage create a third parent into which both P and Frac(ZZ['x'] coerce. I commented accordingly in the new patch version.

comment:36 Changed 7 years ago by SimonKing

Arrgh, I can't even count! I counted the axioms like 1., 3., 4., 5...

Fixed in the new version.

comment:37 follow-up: Changed 7 years ago by dimpase

Would it be possible to include the complete code for MyFrac, as an appendix, with line numbers? Otherwise not all things are illustrated by code, e.g. 'You are encouraged to make your parent “unique”' paragraph lets the reader wonder how exactly this can be accomplished. If there was such an appendix, you could say there 'see lines such-and-such of the appendix'. You could also replace some code snippets with such references then.

comment:38 in reply to: ↑ 37 ; follow-up: Changed 7 years ago by dimpase

Replying to dimpase:

Would it be possible to include the complete code for MyFrac, as an appendix, with line numbers? Otherwise not all things are illustrated by code, e.g. 'You are encouraged to make your parent “unique”' paragraph lets the reader wonder how exactly this can be accomplished. If there was such an appendix, you could say there 'see lines such-and-such of the appendix'. You could also replace some code snippets with such references then.

Oops, the code after the line 'Last, we add a method that returns the characteristic of the field.' inherits from UniqueRepresentation. But it was easy to overlook, as my previous comment shows :) Also, it's hard to see how MyFrac and MyElement fit together without a complete listing...

comment:39 in reply to: ↑ 38 ; follow-up: Changed 7 years ago by SimonKing

Replying to dimpase:

Replying to dimpase:

Would it be possible to include the complete code for MyFrac, as an appendix, with line numbers? Otherwise not all things are illustrated by code, e.g. 'You are encouraged to make your parent “unique”' paragraph lets the reader wonder how exactly this can be accomplished. If there was such an appendix, you could say there 'see lines such-and-such of the appendix'. You could also replace some code snippets with such references then.

Oops, the code after the line 'Last, we add a method that returns the characteristic of the field.' inherits from UniqueRepresentation. But it was easy to overlook, as my previous comment shows :)

But I explicitly state in the text

You are encouraged to make your parent “unique”. That’s to say, parents should only evaluate equal if they are identical. Sage provides frameworks to create unique parents. We use here the most easy one: Inheriting from the class sage.structure.unique_representation.UniqueRepresentation is enough. Making parents unique can be quite important for an efficient implementation, because the repeated creation of “the same” parent would take a lot of time.

Do you really think this is not enough?

Also, it's hard to see how MyFrac and MyElement fit together without a complete listing...

OK. The didactic approach is to develop the example step by step. But I think having the complete code in the appendix would be nice. Is there an automated way to typeset the code with linenumbers?

comment:40 Changed 7 years ago by SimonKing

PS: And a few lines after motivating the use of UniqueRepresentation in the text, there is a fat note saying

Note

UniqueRepresentation automatically provides our class with pickling, preserving the unique parent condition. If we had defined the class in some external module or in an interactive session, pickling would work immediately.

However, for making the example work in Sage’s doctesting framework, we need to assign our class as an attribute of the __main__ module, so that the class can be looked up during unpickling.

I really don't see how this can be overlooked.

Last edited 7 years ago by SimonKing (previous) (diff)

comment:41 in reply to: ↑ 39 ; follow-up: Changed 7 years ago by dimpase

Replying to SimonKing:

Replying to dimpase:

Replying to dimpase:

Would it be possible to include the complete code for MyFrac, as an appendix, with line numbers? Otherwise not all things are illustrated by code, e.g. 'You are encouraged to make your parent “unique”' paragraph lets the reader wonder how exactly this can be accomplished. If there was such an appendix, you could say there 'see lines such-and-such of the appendix'. You could also replace some code snippets with such references then.

Oops, the code after the line 'Last, we add a method that returns the characteristic of the field.' inherits from UniqueRepresentation. But it was easy to overlook, as my previous comment shows :)

But I explicitly state in the text

You are encouraged to make your parent “unique”. That’s to say, parents should only evaluate equal if they are identical. Sage provides frameworks to create unique parents. We use here the most easy one: Inheriting from the class sage.structure.unique_representation.UniqueRepresentation is enough. Making parents unique can be quite important for an efficient implementation, because the repeated creation of “the same” parent would take a lot of time.

Do you really think this is not enough?

The confusion comes from the fact that you explain the code before showing the code (or at least saying that it is coming). There is also an inconsistency in the exposition, as you have the line

sage: from sage.rings.ring import Field

but no similar line for UniqueRepresentation. I guess I might have waited for sage.structure.unique_representation to show up in the code, but it didn't. Call me dyslexic :-).

Also, it's hard to see how MyFrac and MyElement fit together without a complete listing...

OK. The didactic approach is to develop the example step by step. But I think having the complete code in the appendix would be nice. Is there an automated way to typeset the code with linenumbers?

After some trial and error upon reading Sphinx manual, I found that the following will work with sphinx in Sage:

.. highlight:: python
   :linenothreshold: 2

::

   x = 2
   y = [1,2]
   x = y

.. highlight:: python
   :linenothreshold: 22222

the 1st highlight:: turns on line numberings for fragments with 2 or more lines, and the 2nd highlight:: effectively turns it off.

Last edited 7 years ago by dimpase (previous) (diff)

comment:42 in reply to: ↑ 41 Changed 7 years ago by SimonKing

Replying to dimpase:

Do you really think this is not enough?

The confusion comes from the fact that you explain the code before showing the code (or at least saying that it is coming).

OK, then it is similar to pure maths: First show the theorem and then the proof. Actually, I tend to present a chain of thoughts that eventually constitutes the proof of a theorem that is only formulated after the proof. I'll try to change that.

There is also an inconsistency in the exposition, as you have the line

sage: from sage.rings.ring import Field

but no similar line for UniqueRepresentation.

I explicitly imported Field, because this is what one needs to do when writing a module. So, I agree I should do the same with UniqueRepresentation.

After some trial and error upon reading Sphinx manual, I found that the following will work with sphinx in Sage:

.. highlight:: python
   :linenothreshold: 2

::

   x = 2
   y = [1,2]
   x = y

.. highlight:: python
   :linenothreshold: 22222

the 1st highlight:: turns on line numberings for fragments with 2 or more lines, and the 2nd highlight:: effectively turns it off.

Cool, thank you!

comment:43 Changed 7 years ago by SimonKing

The tutorial is updated. It now contains the code twice: In small pieces in the text, and in one chunk, in the appendix.

sage -t passes on the .rst file. When I put the code from the appendix into a .py file and load it into a Sage session, I can do:

sage: %runfile /home/simon/SAGE/work/categories/tutorial.py
sage: Q = MyFrac(ZZ['x'], category=QuotientFieldsWithTest())
sage: TestSuite(Q).run()

comment:44 follow-up: Changed 7 years ago by SimonKing

By the way, it is a bit ironical:

sage: Q.gen()
Traceback (most recent call last):
...
RuntimeError: NewFrac(Univariate Polynomial Ring in x over Integer Ring) still using old coercion framework

I think that this error is totally misleading. Clearly, the new coercion model is used, is it not?

The test against the old coercion model simply asks whether P._element_constructor is not None:

cdef inline check_old_coerce(parent.Parent p):
    if p._element_constructor is not None:
        raise RuntimeError, "%s still using old coercion framework" % p

Since P._element_constructor now seems to coincide with P._element_constructor_, this test is simply wrong. Similarly:

sage: def check_old_coerce(p):
....:     if p._element_constructor is not None:
....:         raise RuntimeError, "%s still using old coercion framework" % p
....:     
sage: check_old_coerce(QQ[['x']])
Traceback (most recent call last):
...
RuntimeError: Power Series Ring in x over Rational Field still using old coercion framework
sage: check_old_coerce(SymmetricFunctionAlgebra(QQ))
Traceback (most recent call last)
...
RuntimeError: Symmetric Functions over Rational Field in the Schur basis still using old coercion framework

I think it is time to remove this test. But not on this ticket, of course.

Last edited 7 years ago by SimonKing (previous) (diff)

comment:45 Changed 7 years ago by SimonKing

I just fixed some typos.

comment:46 Changed 7 years ago by dimpase

Please remove unneeded "===========================" which turn Appendix into a separate document: (that's certainly not what we want, right?)

--- a/doc/en/thematic_tutorials/coercion_and_categories.rst
+++ b/doc/en/thematic_tutorials/coercion_and_categories.rst
@@ -1681,7 +1681,7 @@
 
 .. end of output
 
-===========================
+
 Appendix: The complete code
 ===========================
 

comment:47 follow-up: Changed 7 years ago by dimpase

Could you also add the following little patch:

--- a/doc/en/thematic_tutorials/tutorial-objects-and-classes.rst
+++ b/doc/en/thematic_tutorials/tutorial-objects-and-classes.rst
@@ -1,7 +1,7 @@
 .. _tutorial-objects-and-classes:
 
 ================================================
-Tutorial: Objects and Classes in Python and Sage
+Objects and Classes in Python and Sage
 ================================================
 
 .. MODULEAUTHOR:: Florent Hivert <florent.hivert@univ-rouen.fr>

this fixes the awkwardly standing out "Tutorial:" in tutorial-objects-and-classes.rst and in doc/output/html/en/thematic_tutorials/index.html

Changed 7 years ago by SimonKing

Add a thematic tutorial on categories and coercion

comment:48 in reply to: ↑ 47 Changed 7 years ago by SimonKing

Replying to dimpase:

Could you also add the following little patch: ...

this fixes the awkwardly standing out "Tutorial:" in tutorial-objects-and-classes.rst and in doc/output/html/en/thematic_tutorials/index.html

I hope Florent doesn't mind...

Anyway, I have updated both tutorials, turning the appendix into a section. I also used the occasion to provide my tutorial with a sub-title (since I just learnt how to do those things in sphinx:)

comment:49 in reply to: ↑ 44 ; follow-up: Changed 7 years ago by dimpase

  • Cc vbraun added

Replying to SimonKing:

By the way, it is a bit ironical:

sage: Q.gen()
Traceback (most recent call last):
...
RuntimeError: NewFrac(Univariate Polynomial Ring in x over Integer Ring) still using old coercion framework

I think that this error is totally misleading. Clearly, the new coercion model is used, is it not?

Does this mean that one has to change some code here to take care of Volker's remark on sage-devel ?

comment:50 in reply to: ↑ 49 Changed 7 years ago by SimonKing

Replying to dimpase:

I think that this error is totally misleading. Clearly, the new coercion model is used, is it not?

Does this mean that one has to change some code here to take care of Volker's remark on sage-devel ?

I just see that I forgot to answer.

No, I believe using sage.rings.ring.Ring and friends as base classes is correct. If one wants to use generators (which I don't, in this tutorial), then one must implement it. Namely, the gen() method inherited from ParentWithGens belongs to the old coercion model.

So, no need to change the code here! Still needs review.

comment:51 Changed 7 years ago by dimpase

  • Status changed from needs_review to positive_review

OK, looks good enough.

comment:52 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.7 to sage-5.8

comment:53 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.8.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:54 Changed 7 years ago by dimpase

  • Reviewers changed from Vincent Delecroix, Travis Scrimshaw to Vincent Delecroix, Travis Scrimshaw, Dmitrii Pasechnik
Note: See TracTickets for help on using tickets.