Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZZ → QQ is not onto #23186

Closed
saraedum opened this issue Jun 9, 2017 · 11 comments
Closed

ZZ → QQ is not onto #23186

saraedum opened this issue Jun 9, 2017 · 11 comments

Comments

@saraedum
Copy link
Member

saraedum commented Jun 9, 2017

Currently, this fails

sage: QQ.coerce_map_from(ZZ).is_surjective()
NotImplementedError

To fix this, we make the coercion morphism know it is not surjective.

Component: commutative algebra

Keywords: sd86.5

Author: Julian Rüth

Branch/Commit: 79fd62a

Reviewer: Travis Scrimshaw

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

@tscrim
Copy link
Collaborator

tscrim commented Jun 9, 2017

comment:1

You should never, ever give information about specific objects at the category level.

However, you can impart this information in the Z_to_Q morphism class.

@saraedum
Copy link
Member Author

saraedum commented Jun 9, 2017

comment:2

Why not do that at the category level?

@saraedum
Copy link
Member Author

Branch: u/saraedum/zz___qq_is_not_onto

@saraedum
Copy link
Member Author

Author: Julian Rüth

@saraedum
Copy link
Member Author

Changed branch from u/saraedum/zz___qq_is_not_onto to none

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2017

comment:5

Because to do something at the category level, you should be doing something for all objects in that category, not for one specific object (in a specific implementation). Now, if you wanted to have something in Rings checking if a morphism to ZZ was surjective returning True, then that would be good because it is true for all rings (of characteristic 0). Although map has a concrete implementation of is_surjective, which is a technical detail that would need to be worked around (which that method probably should be lifted to the category of set morphisms).

The fix LGTM.


New commits:

79fd62aZZ is not onto QQ

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2017

Branch: u/saraedum/zz___qq_is_not_onto

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2017

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jun 10, 2017

Commit: 79fd62a

@vbraun
Copy link
Member

vbraun commented Jun 15, 2017

Changed branch from u/saraedum/zz___qq_is_not_onto to 79fd62a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants