Use the right category for DefaultConvertMaps, rather than SetsWithPartialMaps

[roed314]

Currently, the category for DefaultConvertMaps is SetsWithPartialMaps().

sage: type(QQ[['x']].coerce_map_from(QQ))
<type 'sage.structure.coerce_maps.DefaultConvertMap_unique'>
sage: QQ[['x']].coerce_map_from(QQ).category_for()
Category of sets with partial maps

In contrast,

sage: QQ.hom(QQ[['x']]).category_for()
Category of euclidean domains

Depends on #23201

Component: coercion

Keywords: sd86.5

Author: David Roe, Julian Rüth

Branch: 5eccb52

Reviewer: Julian Rüth, David Roe

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

[roed314]

Changed keywords from none to sd86.5

[roed314]

Dependencies: #23201

[pjbruin]

comment:2

Please make sure that you only refine the category when this makes mathematical sense. In #15618, I implemented taking SetsWithPartialMaps as the default category for a DefaultConvertMap because in general such a map is not defined everywhere, nor is it always a morphism in the category where the parents live.

[pjbruin]

comment:3

I guess this should be fixed in PowerSeries_generic._coerce_map_from_() by returning a specific map instead of True.

[roed314]

comment:4

Replying to @pjbruin:

Please make sure that you only refine the category when this makes mathematical sense. In #15618, I implemented taking SetsWithPartialMaps as the default category for a DefaultConvertMap because in general such a map is not defined everywhere, nor is it always a morphism in the category where the parents live.

My plan is to only change the category for coercions, not conversions. I can't think of any examples where coercions aren't morphisms in the meet of the categories of the parents involved. All of the examples I see in #15618 are conversions, which this ticket won't affect.

[roed314]

comment:5

Replying to @pjbruin:

I guess this should be fixed in PowerSeries_generic._coerce_map_from_() by returning a specific map instead of True.

You could fix it in this way, but that's a lot of work that would need to be duplicated over and over again for different examples of DefaultConvertMaps.

[roed314]

Branch: u/roed/use_the_right_category_for_defaultconvertmaps__rather_than_setswithpartialmaps

[roed314]

Author: David Roe

[roed314]

Commit: 3cc08d1

[roed314]

New commits:

b00d97f	Make DefaultConvertMaps have a more refined category (than SetsWithPartialMaps) when they're actually coercions
5fbc34c	Change base ring for CuspFormsRing in Hecke triangle group
906d5b2	Fix base rings for many other parents in sage/modular/moform_hecketriangle
3e0fcd7	Merge branch 't/23201/hecke_triangle_group_cusp_form_base_ring' into t/23184/use_the_right_category_for_defaultconvertmaps__rather_than_setswithpartialmaps
3cc08d1	Fix a problem in quivers from changing the category of DefaultConvertMaps, add doctests to _generic_convert_map

[roed314]

comment:8

All tests pass.

[sagetrac-git]

Changed commit from 3cc08d1 to 09d3456

[sagetrac-git]

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

09d3456

Fix a dangling sage: prompt

[saraedum]

comment:10

Looks good except for a few places where S.category() should be replaced with an appropriate meet. I'll fix that myself.

[saraedum]

Changed branch from u/roed/use_the_right_category_for_defaultconvertmaps__rather_than_setswithpartialmaps to u/saraedum/use_the_right_category_for_defaultconvertmaps__rather_than_setswithpartialmaps

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5eccb52

Refactor _default_convert_map

[sagetrac-git]

Changed commit from 09d3456 to 5eccb52

[saraedum]

Changed author from David Roe to David Roe, Julian Rüth

[saraedum]

Reviewer: Julian Rüth

[saraedum]

comment:14

tests pass.

[roed314]

Changed reviewer from Julian Rüth to Julian Rüth, David Roe

[roed314]

comment:15

Looks good.

[vbraun]

Changed branch from u/saraedum/use_the_right_category_for_defaultconvertmaps__rather_than_setswithpartialmaps to 5eccb52

[fchapoton]

Changed commit from 5eccb52 to none

[fchapoton]

comment:17

you used a bad syntax for the trac role here :

+        We check that `trac`:23184 has been resolved::

it should have been

+        We check that :trac:`23184` has been resolved::

So please review #23526