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: | sage-9.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 follow-up: 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()