Opened 3 years ago
Closed 3 years ago
#29145 closed enhancement (fixed)
Indeterminacy locus and image of morphisms of schemes
Reported by:  klee  Owned by:  

Priority:  minor  Milestone:  sage9.1 
Component:  algebraic geometry  Keywords:  
Cc:  Merged in:  
Authors:  Kwankyu Lee  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  7ad46c0 (Commits, GitHub, GitLab)  Commit:  7ad46c06e157a9fdc464405630c1b110de9a90b4 
Dependencies:  Stopgaps: 
Description (last modified by )
Implemented indeterminacy locus and image of morphisms of affine and projective schemes.
Along the way, cleaned up some files in schemes module.
The author was supported by NRF of Korea 2019, 2020
Change History (16)
comment:1 Changed 3 years ago by
Branch:  → u/klee/29145 

comment:2 Changed 3 years ago by
Commit:  → 17aec4e47066e9ec0ebfa9a39086f07d6daa5eee 

comment:3 Changed 3 years ago by
Status:  new → needs_review 

comment:4 Changed 3 years ago by
Authors:  → Kwankyu Lee 

comment:5 Changed 3 years ago by
Description:  modified (diff) 

comment:6 Changed 3 years ago by
Commit:  17aec4e47066e9ec0ebfa9a39086f07d6daa5eee → 721b23ecf740dc6fd6f14b3b1049d881ad58fec4 

Branch pushed to git repo; I updated commit sha1. New commits:
721b23e  Fixes for doctest failures

comment:7 Changed 3 years ago by
Commit:  721b23ecf740dc6fd6f14b3b1049d881ad58fec4 → 5c49204281bce6053fa52b3187c72b610aee74f9 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5c49204  Added indeterminacy_locus() and image()

comment:8 Changed 3 years ago by
Commit:  5c49204281bce6053fa52b3187c72b610aee74f9 → aded9e3da7e46de08ac92b90efa0a3af58ac5cf9 

Branch pushed to git repo; I updated commit sha1. New commits:
aded9e3  Added a reference

comment:9 Changed 3 years ago by
Commit:  aded9e3da7e46de08ac92b90efa0a3af58ac5cf9 → 4ea25f2759580b3bfabbc4a3c02eb2981bc3d8fb 

Branch pushed to git repo; I updated commit sha1. New commits:
4ea25f2  A pyflakes fix

comment:10 Changed 3 years ago by
Commit:  4ea25f2759580b3bfabbc4a3c02eb2981bc3d8fb → 383b618a6400b1f69a3c447e6536841c9143280e 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
383b618  Added indeterminacy_locus() and image()

comment:11 followup: 12 Changed 3 years ago by
A few things:
It would be good to have a more clear indication for this list here
sage: g.representatives() [Scheme morphism: From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by: x^2  y^2  y To: Affine Space of dimension 1 over Rational Field Defn: Defined on coordinates by sending (x, y) to  (x/y), Scheme morphism: + (x/y), + Scheme morphism: From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by: x^2  y^2  y To: Affine Space of dimension 1 over Rational Field Defn: Defined on coordinates by sending (x, y) to ((y + 1)/x)]
and similar changes for the other tests in this method.
Do you need to move the SchemeMorphism.__init__(self, parent)
in SchemeMorphism_polynomial.__init__
? I feel it would be better to do it after the possible error messages for when those are raised so as to not do unnecessary work (granted, if it is needed to be initialized for the tests, then this comment is moot).
PEP8:
p = source_ring(poly.numerator())/source_ring(poly.denominator()) +p = source_ring(poly.numerator()) / source_ring(poly.denominator())
I don't understand this bit of code:
if not self.codomain().is_projective(): pass
There is no else
statement attached, so I feel it can be removed.
Is the Chow_form
still usable by instances of AlgebraicScheme_subscheme_projective
? Are they suppose to be?
comment:12 Changed 3 years ago by
Replying to tscrim:
A few things:
It would be good to have a more clear indication for this list here
sage: g.representatives() [Scheme morphism: From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by: x^2  y^2  y To: Affine Space of dimension 1 over Rational Field Defn: Defined on coordinates by sending (x, y) to  (x/y), Scheme morphism: + (x/y), + Scheme morphism: From: Closed subscheme of Affine Space of dimension 2 over Rational Field defined by: x^2  y^2  y To: Affine Space of dimension 1 over Rational Field Defn: Defined on coordinates by sending (x, y) to ((y + 1)/x)]and similar changes for the other tests in this method.
OK. Done.
Do you need to move the
SchemeMorphism.__init__(self, parent)
inSchemeMorphism_polynomial.__init__
? I feel it would be better to do it after the possible error messages for when those are raised so as to not do unnecessary work (granted, if it is needed to be initialized for the tests, then this comment is moot).
No. You are right. I wrongly thought it is needed. Moved back.
PEP8:
p = source_ring(poly.numerator())/source_ring(poly.denominator()) +p = source_ring(poly.numerator()) / source_ring(poly.denominator())
Done.
I don't understand this bit of code:
if not self.codomain().is_projective(): passThere is no
else
statement attached, so I feel it can be removed.
This part was "to do", and then I somehow forgot.. Now I filled the missing part, which is for morphisms from projective subscheme to affine space. Thank you for spotting this!
Is the
Chow_form
still usable by instances ofAlgebraicScheme_subscheme_projective
? Are they suppose to be?
Chow_form
method assumes the base ring is a field. Methods of AlgebraicScheme_subscheme_projective
should assume just rings. So it would be usable when the assumption is met.
comment:13 Changed 3 years ago by
Commit:  383b618a6400b1f69a3c447e6536841c9143280e → 213ff8ffabd942b37aab704371ceb1695629ea67 

Branch pushed to git repo; I updated commit sha1. New commits:
213ff8f  Fixes for reviewer comments

comment:14 Changed 3 years ago by
Commit:  213ff8ffabd942b37aab704371ceb1695629ea67 → 7ad46c06e157a9fdc464405630c1b110de9a90b4 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
7ad46c0  Fixes for reviewer comments

comment:15 Changed 3 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
Thank you for the changes and explanations. LGTM.
comment:16 Changed 3 years ago by
Branch:  u/klee/29145 → 7ad46c06e157a9fdc464405630c1b110de9a90b4 

Resolution:  → fixed 
Status:  positive_review → closed 
Branch pushed to git repo; I updated commit sha1. New commits:
Added indeterminacy_locus() and image()