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

Replace is_IntegerModRing by isinstance with new class sage.rings.abc.IntegerModRing #32606

Closed
mkoeppe opened this issue Oct 1, 2021 · 18 comments
Closed

Replace is_IntegerModRing by isinstance with new class sage.rings.abc.IntegerModRing #32606

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

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2021

(partially cherry-picked from #32432)

Part of Meta-ticket #32414

Depends on #32593

CC: @tscrim @fchapoton

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 907b57c

Reviewer: Travis Scrimshaw

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

mkoeppe commented Oct 1, 2021

Branch: u/mkoeppe/replace_is_integermodring_by_isinstance_with_new_class_sage_rings_abc_integermodring

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

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

335cd3eReplace all uses of is_IntegerModRing by isinstance(..., sage.rings.abc.IntegerModRing)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

Commit: 335cd3e

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 1, 2021

Author: Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

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

f596395sage.rings.finite_rings.integer_mod_ring: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

Changed commit from 335cd3e to f596395

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 1, 2021

comment:7

Should the is_... function be deprecated?

@tscrim
Copy link
Collaborator

tscrim commented Oct 1, 2021

comment:8

I think we can just remove it altogether. It has been semi-officially deprecated and is only used internally (well, supposed to be). Perhaps better practice is to formally deprecate it though. So +1 to at least a deprecation.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

Changed commit from f596395 to af06e5b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 1, 2021

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

af06e5bis_IntegerModRing: Deprecate

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2021

comment:10

One more is_* function down. Thank you.

@tscrim
Copy link
Collaborator

tscrim commented Oct 3, 2021

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Oct 11, 2021

comment:11

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

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

1b0a5absrc/sage/modules/vector_space_morphism.py: Do not fail if sage.symbol is not present
83b5038src/sage/modules/free_module_element.pyx: Move import from sage.misc.derivative into method
acca6c2src/sage/matrix/matrix2.pyx: Move import from sage.misc.derivative into method
907b57cMerge #32593

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 11, 2021

Changed commit from af06e5b to 907b57c

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 11, 2021

Changed dependencies from #32566 to #32593

@vbraun
Copy link
Member

vbraun commented Oct 13, 2021

Changed branch from u/mkoeppe/replace_is_integermodring_by_isinstance_with_new_class_sage_rings_abc_integermodring to 907b57c

@vbraun vbraun closed this as completed in d694413 Oct 13, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 21, 2023
@mkoeppe
src/sage/rings/finite_rings/integer_mod_ring.py: Remove function depr…
29401c6 
…ecated in sagemath#32606 (2021)
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
sagemathgh-36307: sage.rings: Remove code deprecated in sagemath#23204
9ea5127 
, 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
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