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

add level parameter to rational_preimages for projective points #20780

Closed
bhutz opened this issue Jun 7, 2016 · 12 comments
Closed

add level parameter to rational_preimages for projective points #20780

bhutz opened this issue Jun 7, 2016 · 12 comments

Comments

@bhutz
Copy link

bhutz commented Jun 7, 2016

When working with preimage trees it is usefull to compute more than just the first preimages of a given point. The ticket is to add a parameter to rational_preimages() to specify how far back to find preimages. I.e. find all points Q with f^k(Q) =P for specified k.

Component: algebraic geometry

Author: Ben Hutz

Branch/Commit: 6377a27

Reviewer: Grayson Jorgenson, Rebecca Lauren Miller

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

@bhutz bhutz added this to the sage-7.3 milestone Jun 7, 2016
@bhutz bhutz self-assigned this Jun 7, 2016
@bhutz
Copy link
Author

bhutz commented Jun 7, 2016

Commit: e7db093

@bhutz
Copy link
Author

bhutz commented Jun 7, 2016

Author: Ben Hutz

@bhutz
Copy link
Author

bhutz commented Jun 7, 2016

New commits:

e7db09320780: add level parameter to rational_preimages

@bhutz
Copy link
Author

bhutz commented Jun 7, 2016

Branch: u/bhutz/t/20780

@sagetrac-gjorgenson
Copy link
Mannequin

sagetrac-gjorgenson mannequin commented Jun 8, 2016

Reviewer: Grayson Jorgenson

@sagetrac-gjorgenson
Copy link
Mannequin

sagetrac-gjorgenson mannequin commented Jun 8, 2016

comment:2

I think this looks good. The code makes sense and appears to work as intended. I just found some really minor things:

-algebraic_scheme.py

  • line 2780 - change TypeError message to all lowercase
  • Old issues present before this ticket that maybe could be addressed here:
  • line 2680 - `False` to ``False``
  • line 2711 - some spacing in ProjectiveSpace calls and in lists of defining polynomials

-projective_homset.py

  • line 182 - the sorted() function takes any iterable and returns a list, so it shouldn't be necessary to convert rat_points to a list first

-projective_morphism.py

  • line 3920 spacing around '>'
  • lines 3790 - 3793, formatting issues: ``k``th and `k`th don't seem to render well, and perhaps
    `P` and `Q` should be replaced with ``P``, ``Q``.
  • some ProjectiveSpace calls in examples of rational_preimages don't have any spacing in the arguments

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2016

Changed commit from e7db093 to 6377a27

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2016

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

6377a2720780: minor fixes

@sagetrac-gjorgenson
Copy link
Mannequin

sagetrac-gjorgenson mannequin commented Jun 8, 2016

comment:4

Okay, this looks fine to me now.

@sagetrac-rlmiller
Copy link
Mannequin

sagetrac-rlmiller mannequin commented Jun 11, 2016

comment:5

Looks good to me.

@sagetrac-rlmiller
Copy link
Mannequin

sagetrac-rlmiller mannequin commented Jun 11, 2016

Changed reviewer from Grayson Jorgenson to Grayson Jorgenson, Rebecca Lauren Miller

@vbraun
Copy link
Member

vbraun commented Jun 12, 2016

Changed branch from u/bhutz/t/20780 to 6377a27

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

2 participants