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

Deprecate is_[Callable]SymbolicExpressionRing, remove use of is_Symbolic{Equation,Variable}, is_CallableSymbolicExpression #32665

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

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 10, 2021

We deprecate the functions is_SymbolicExpressionRing and is_CallableSymbolicExpressionRing and replace all uses by isinstance with new ABCs sage.rings.abc.SymbolicRing, sage.rings.abc.CallableSymbolicExpressionRing.

As a follow-up on #32638, which deprecated is_Expression, we remove uses of is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable outside of sage.symbolic, sage.calculus, sage.interfaces.maxima_lib.

We replace these uses by isinstance(x, Expression) and a method call. We add a new method Expression.is_callable.

This is part of meta-ticket #32414.

Depends on #32638
Depends on #32593
Depends on #32606
Depends on #32612

CC: @tscrim @orlitzky @kliem

Component: symbolics

Author: Matthias Koeppe

Branch: 341337a

Reviewer: Travis Scrimshaw

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

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

mkoeppe commented Oct 17, 2021

Dependencies: #32638

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

Changed dependencies from #32638 to #32638, #32593

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

Changed dependencies from #32638, #32593 to #32638, #32593, #32606

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

Changed dependencies from #32638, #32593, #32606 to #32638, #32593, #32606, #32612

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Deprecate is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable Deprecate is_[Callable]SymbolicExpressionRing and is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable Oct 17, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

comment:7

Setting to "needs_review" so that the patchbot runs


Last 10 new commits:

fc039f1src/sage/symbolic/ring.pyx: Fix imports
19ac6fdsrc/sage/rings/complex_interval_field.py: Fixup deprecation warning in doctest output
f31deb3src/sage/rings/qqbar.py: Remove unused import
362f5c0Merge branch 't/32606/replace_is_integermodring_by_isinstance_with_new_class_sage_rings_abc_integermodring' into t/32612/sage_rings_abc__real_complex__interval_ball_field__deprecate_is__real_complex_intervalfield
355dbcbMerge #32606
fb56937Merge tag '9.5.beta3' into t/32610/deprecate_is_realfield__is_complexfield__is_realdoublefield__is_complexdoublefield
8cc3500src/sage/rings/polynomial/polynomial_singular_interface.py: Fixup merge
124f213Merge #32610
80eff2aMerge #32612
5fb862eDeprecate is_[Callable]SymbolicExpressionRing, replace uses by isinstance(..., sage.rings.abc....)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 17, 2021

Commit: 5fb862e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from 5fb862e to ee4bd52

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

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

ee4bd52sage.rings.abc, sage.symbolic.ring: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from ee4bd52 to 4bc059b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

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

b484d51src/sage/symbolic/callable.py: Fixup
fad87c0Expression.is_callable: New
4bc059bsrc/sage/ext/fast_callable.pyx: Remove use of is_CallableSymbolicExpression

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from 4bc059b to 37da733

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

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

8624925src/sage/symbolic/ring.pyx: Update doctest output with deprecation warning
37da733src/sage/sets/condition_set.py: Remove use of is_CallableSymbolicExpression

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

Changed commit from 37da733 to 341337a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 17, 2021

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

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

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Deprecate is_[Callable]SymbolicExpressionRing and is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable Deprecate is_[Callable]SymbolicExpressionRing, remove use of is_Symbolic{Equation,Variable}, is_CallableSymbolicExpression Oct 17, 2021
@tscrim
Copy link
Collaborator

tscrim commented Oct 18, 2021

comment:13

If this is fully ready for review (just checking relative to comment:7), then you can set a positive review.

@tscrim
Copy link
Collaborator

tscrim commented Oct 18, 2021

Reviewer: Travis Scrimshaw

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 18, 2021

comment:14

Thank you! Yes, I was finished here.

@vbraun
Copy link
Member

vbraun commented Oct 20, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 24, 2022

comment:16

Follow up in #34215

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 24, 2022

Changed commit from 341337a to none

mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 17, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 19, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Oct 11, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 17, 2023
…h#18036, sagemath#29738, sagemath#32386, sagemath#32638, sagemath#32665, sagemath#34215

    
<!-- ^^^^^
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#36304
Reported by: Matthias Köppe
Reviewer(s): David Coudert
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 19, 2023
…h#18036, sagemath#29738, sagemath#32386, sagemath#32638, sagemath#32665, sagemath#34215

    
<!-- ^^^^^
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#36304
Reported by: Matthias Köppe
Reviewer(s): David Coudert
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

3 participants