Open 2889fc47-50c1-4c1a-bb91-f80beb6916b6 opened 3 years ago
Description changed:
---
+++
@@ -1,2 +1,2 @@
we modified the if so that it doesn't throw an error when working with
- dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field
+dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field
Description changed:
---
+++
@@ -1,2 +1 @@
-we modified the if so that it doesn't throw an error when working with
-dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field
+we modified the if condition so that it doesn't throw an error when working with dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.
Commit: 330a6d0
New commits:
330a6d0 | we modified the if condition so that it doesn't throw an error when working with dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field |
I can take a look at this change, but it probably won't be until next week.
just a quick comment. The commit message should be short (one line) with the more detailed description in a following paragraph.
Reviewer: Ben Hutz
On the surface this functionality works, but the details need a little work.
In the code itself there are a number of white space issues. For example
[(1/t)*x^2 + y^2 ,y^2])
should be
[(1/t)*x^2 + y^2, y^2])
In the new if line as well, you're missing a space after the comma and have an extra space before the closing ).
The example code should be improved in a number of ways:
While adding support for function fields, seems like you should also allow finite fields, since that also requires no other changes.
Branch pushed to git repo; I updated commit sha1. New commits:
46fc21e | fixed test |
There is a doctest failure. Looks like you might need to use "sorted" for that doctest.
Also, why did you not also allow finite fields as requested?
The whitespace issues were fixed in the example, but the example no longer is a true function field example as the function and all the points are over the base field. The example should use the function field variable 't'.
Setting a new milestone for this ticket based on a cursory review.
we modified the if condition so that it doesn't throw an error when working with dynamical systems over function field the actual code of the function is not changed because it works on dynamical systems over function field
CC: @bhutz
Component: dynamics
Author: Saher Amasha,Safa Amasha
Branch/Commit: u/gh-Saher-Amasha/add_support_for_dynamical_systems_over_function_field_in_function__all_rational_preimages @
46fc21e
Reviewer: Ben Hutz
Issue created by migration from https://trac.sagemath.org/ticket/31549