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

sage.rings.abc: Add pAdicRing, pAdicField; deprecate is_pAdic... #32750

Closed
mkoeppe opened this issue Oct 24, 2021 · 30 comments
Closed

sage.rings.abc: Add pAdicRing, pAdicField; deprecate is_pAdic... #32750

mkoeppe opened this issue Oct 24, 2021 · 30 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 24, 2021

This change will allow use to eliminate imports from sage.rings.padics in sage.rings.polynomial.polynomial_element.

Part of Meta-ticket #32414.

Depends on #32665

CC: @EnderWannabe @bhutz

Component: refactoring

Author: Matthias Koeppe, Alexander Galarraga

Branch/Commit: 03b24bd

Reviewer: Jonathan Kliem

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 24, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

Commit: e1412c7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

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

1893620src/sage/dynamics/arithmetic_dynamics/projective_ds.py: Use sage.rings.abc.pAdicRing, pAdicField
8422015src/sage/modular/overconvergent/genus0.py: Use sage.rings.abc.pAdicField
c32b3a2src/sage/schemes/berkovich: Use sage.rings.abc.pAdicField
96a778bsrc/sage/schemes/elliptic_curves/constructor.py: Use sage.rings.abc.pAdicField
b672836src/sage/schemes/hyperelliptic_curves: Use sage.rings.abc.pAdicField
e1412c7src/sage/schemes/projective/projective_morphism.py: Use sage.rings.abc.pAdicField

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

Author: Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

Changed commit from e1412c7 to fc7025a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

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

fc7025asrc/sage/rings/padics/generic_nodes.py: Update doctests with deprecation warning output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

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

070d05bsrc/sage/schemes/projective/projective_morphism.py: Avoid merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 24, 2021

Changed commit from fc7025a to 070d05b

@EnderWannabe
Copy link
Contributor

Changed commit from 070d05b to 59271be

@EnderWannabe
Copy link
Contributor

comment:7

Added missing imports in src/sage/modular/overconvergent/genus0.py and src/sage/schemes/hyperelliptic_curves/constructor.py. Hopefully this passes patchbot now; the rest of the changes looked good to me.


New commits:

59271be32750: added missing imports

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

comment:8

Thanks for fixing this!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 24, 2021

comment:9

The remaining failure, from src/sage/rings/integer.pyx, is not from this ticket. It is fixed in #32737.

@kliem
Copy link
Contributor

kliem commented Oct 25, 2021

comment:10

pyflakes detects that the import from sage.rings.ring import EuclideanDomain, Field in generic_nodes.py can probably be removed now.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2021

Changed commit from 59271be to 0a1b9ac

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

59271be32750: added missing imports
0a1b9acsrc/sage/rings/padics/generic_nodes.py: Remove unused imports

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 25, 2021

comment:13

Rebased on top of gh-EnderWannabe's fixes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2021

Changed commit from 0a1b9ac to c46eb46

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2021

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

b484d51src/sage/symbolic/callable.py: Fixup
fad87c0Expression.is_callable: New
4bc059bsrc/sage/ext/fast_callable.pyx: Remove use of is_CallableSymbolicExpression
8624925src/sage/symbolic/ring.pyx: Update doctest output with deprecation warning
37da733src/sage/sets/condition_set.py: Remove use of is_CallableSymbolicExpression
80a8f9esage.plot: Remove use of is_CallableSymbolicExpression, is_SymbolicEquation
a287531src/sage/schemes/elliptic_curves/constructor.py: Remove use of SR, is_SymbolicEquation; add test for symbolic input
c9861d1src/sage/interfaces/qepcad.py: Remove use of is_SymbolicEquation
341337asrc/sage/ext/fast_callable.pyx: Remove use of is_SymbolicVariable
c46eb46Merge #32665

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 25, 2021

comment:15

Merged #32665 to resolve merge conflict.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 25, 2021

Dependencies: #32665

@kliem
Copy link
Contributor

kliem commented Oct 26, 2021

comment:16

If we are touching this line anyway, we might as well add the second space before #:

+        elif isinstance(base, sage.rings.abc.pAdicField): # change base to Qpbar

Once done, you can put it on positive review on my behalf.

@kliem
Copy link
Contributor

kliem commented Oct 26, 2021

Reviewer: Jonathan Kliem

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2021

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

03b24bdsrc/sage/schemes/berkovich/berkovich_space.py: Whitespace fix

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 26, 2021

Changed commit from c46eb46 to 03b24bd

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 26, 2021

Changed author from Matthias Koeppe to Matthias Koeppe, Alexander Galarraga

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 26, 2021

comment:18

Thank you!

@vbraun
Copy link
Member

vbraun commented Oct 31, 2021

@vbraun vbraun closed this as completed in caa4d18 Oct 31, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
, sagemath#24483, sagemath#24371, sagemath#24511, sagemath#25848, sagemath#26105, sagemath#28481, sagemath#29010, sagemath#29412, sagemath#30332, sagemath#30372, sagemath#31345, sagemath#32375, sagemath#32606, sagemath#32610, sagemath#32612, sagemath#32641, sagemath#32660, sagemath#32750, sagemath#32869, sagemath#33602

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36307
Reported by: Matthias Köppe
Reviewer(s):
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

4 participants