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.

Sign up for GitHub

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

Jump to bottom

sage.structure.element: Add abc Expression; deprecate is_Expression etc. #32638

Closed
mkoeppe opened this issue Oct 5, 2021 · 34 comments
Closed

sage.structure.element: Add abc Expression; deprecate is_Expression etc. #32638

mkoeppe opened this issue Oct 5, 2021 · 34 comments
Labels
p: major / 3 t: enhancement t: refactoring
Milestone
sage-9.5

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Oct 5, 2021

(part of meta-ticket #32414)

in sage.structure.element, we add a class Expression

  • the existing class sage.symbolic.expression.Expression will be its only subclass
  • deprecate is_Expression, replace uses by isinstance(x, sage.structure.element.Expression)

Follow-up in #32665: Also deprecate is_SymbolicEquation, is_CallableSymbolicExpression, is_SymbolicVariable; replace uses by isinstance(x, Expression) and a method call (may need a new method is_callable)

Depends on #32599

CC: @DaveWitteMorris @tscrim @mjungmath

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: ee7640c

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32638
@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 5, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 5, 2021

Branch: u/mkoeppe/sage_structure_element__add_abc_expression__deprecate_is_expression_etc_

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 5, 2021

New commits:

37ee682sage.structure.element.Expression: New abc for sage.symbolic.expression.Expression

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 5, 2021

Commit: 37ee682

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

Changed commit from 37ee682 to 08bf900

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 5, 2021

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

56d1c36git grep -l -E 'is_Expression' | xargs sed -E -i.bak 's/sage[.]symbolic.*import is_Expression *$/sage.structure.element import Expression/;s/is_Expression[(]([^)]*)[)]/isinstance(\1, Expression)/g;'
08bf900is_Expression: Undo automatic edit of the definition; deprecate

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 5, 2021

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 5, 2021

comment:4

setting to needs_review so that the patchbot runs

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

4e5fe35src/sage/symbolic/expression.pxd: Undo automatic edit

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from 08bf900 to 4e5fe35

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

e14259bsrc/sage/ext/fast_callable.pyx: Fix up clash of sage.ext.fast_callable.Expression and sage.structure.element.Expression

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from 4e5fe35 to e14259b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

daed11esrc/sage/symbolic/function.pyx: Update imports

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from e14259b to daed11e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from daed11e to c52251b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

c52251bsrc/sage/symbolic/ring.pyx: Update imports

@mkoeppe

This comment has been minimized.

Sign in to view
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from c52251b to 2a81e0f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

0f30530src/sage/misc/sageinspect.py: Fix up a doctest on the source code of src/sage/symbolic/expression.pyx
2a81e0fsrc/sage/symbolic/expression.pyx: Fix markup of doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

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

88886eesrc/sage/symbolic/expression.pyx: Fix doctest output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 6, 2021

Changed commit from 2a81e0f to 88886ee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

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

cf9b75asrc/sage/symbolic/relation.py: Remove unused import; use sage.structure.element.Expression for isinstance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2021

Changed commit from 88886ee to cf9b75a

@tscrim
Copy link
Collaborator

tscrim commented Oct 7, 2021

comment:14

LGTM.

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 7, 2021

comment:15

Thank you!

@mkoeppe

This comment has been minimized.

Sign in to view
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

ec8400bMerge tag '9.5.beta3' into t/32638/sage_structure_element__add_abc_expression__deprecate_is_expression_etc_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Changed commit from cf9b75a to ec8400b

@mkoeppe
Copy link
Member Author

mkoeppe commented Oct 13, 2021

Dependencies: #32599

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2021

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

8e27fdcsrc/sage/structure/sequence.py: Do not fail if polynomial rings cannot be imported
b43a50csrc/sage/sets/condition_set.py: Do not fail if sage.symbolic cannot be imported
73aabd2src/sage/sets/condition_set.py: Fix up import
ee7640cMerge #32599

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 13, 2021

Changed commit from ec8400b to ee7640c

@vbraun
Copy link
Member

vbraun commented Oct 19, 2021

Changed branch from u/mkoeppe/sage_structure_element__add_abc_expression__deprecate_is_expression_etc_ to ee7640c

@vbraun vbraun closed this as completed in 9d8af60 Oct 19, 2021
This was referenced Jan 30, 2023
Open
Open
Closed
Closed
Closed
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
@mkoeppe
Merge sagemath#32638
687ed9a
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 19, 2023
@mkoeppe
sage.symbolic.expression: Remove function deprecated in sagemath#32638 …
9b9008b 
…(2021)
mkoeppe added a commit to mkoeppe/sage that referenced this issue Oct 11, 2023
@mkoeppe
sage.symbolic.expression: Remove function deprecated in sagemath#32638 …
cdfbdca 
…(2021)
vbraun pushed a commit to vbraun/sage that referenced this issue Oct 17, 2023
sagemathgh-36304: sage.symbolics: Remove code deprecated in sagemat…
52011ee 
…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
sagemathgh-36304: sage.symbolics: Remove code deprecated in sagemat…
e3e4288 
…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
Assignees
No one assigned
Labels
p: major / 3 t: enhancement t: refactoring
Projects
None yet
Milestone
sage-9.5
Development

No branches or pull requests
3 participants
@vbraun @tscrim @mkoeppe