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

New ABC sage.structure.element.NumberFieldElement, deprecate is_NumberFieldElement #34931

Closed
mkoeppe opened this issue Jan 23, 2023 · 14 comments · Fixed by #35100
Closed

New ABC sage.structure.element.NumberFieldElement, deprecate is_NumberFieldElement #34931

mkoeppe opened this issue Jan 23, 2023 · 14 comments · Fixed by #35100
Labels
p: major / 3 t: enhancement t: refactoring
Milestone
sage-10.0

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jan 23, 2023 •
edited
Loading

We also make sage.rings.number_field a namespace package. This is necessary because .number_field_base is needed in sagemath-categories (#32432), whereas implementation modules depend on NTL.

Part of Meta-ticket #32414

CC: @tscrim

Component: refactoring

Author: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/34931
@mkoeppe mkoeppe added this to the sage-9.8 milestone Jan 23, 2023
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 24, 2023

Branch: u/mkoeppe/new_abc_sage_structure_element_numberfieldelement__deprecate_is_numberfieldelement

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2023

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

a9dacbeis_NumberFieldElement: Deprecate, replace uses by isinstance

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2023

Commit: a9dacbe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2023

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

1cc09ccReplace some imports of NumberFieldElement classes by ABC
4dfefbfsrc/sage/rings/number_field/number_field_element.pyx: Update doctest with deprecation warning

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 24, 2023

Changed commit from a9dacbe to 4dfefbf

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 24, 2023

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 25, 2023

comment:5

Instead of adding the ABCDEFIT NumberFieldElement to sage.structure.element, we could also create a tiny new module sage.rings.number_field.number_field_element_base for it. Not sure which of the two is preferable.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2023

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

bacff96src/sage/rings/number_field/__init__.py: Remove

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 25, 2023

Changed commit from 4dfefbf to bacff96

@mkoeppe

This comment has been minimized.

Sign in to view
@mkoeppe

This comment has been minimized.

Sign in to view
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 26, 2023

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

3e95ebfsrc/sage/structure/element.pyx: Fix up doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 26, 2023

Changed commit from bacff96 to 3e95ebf

@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 29, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
@mkoeppe
Merge sagemath#34931
81e875b
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
@mkoeppe
Merge sagemath#34931
62da507
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2023

Removed branch from issue description; replaced by PR #35100

vbraun pushed a commit that referenced this issue Mar 19, 2023
gh-35100: New ABC sage.structure.element.NumberFieldElement, deprecat…
357ca10 
…e is_NumberFieldElement

    
<!-- ^^^^^
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 #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description
Fixes #34931

<!-- Describe your changes here in detail -->
<!-- Why is this change required? What problem does it solve? -->
<!-- If it resolves an open issue, please link to the issue here. For
example "Closes #1337" -->

### 📝 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! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->
<!--
- #xyz: short description why this is a dependency
- #abc: ...
-->
Depends on #35033
    
URL: #35100
Reported by: Matthias Köppe
Reviewer(s): Matthias Köppe, Travis Scrimshaw
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-10.0
Development

Successfully merging a pull request may close this issue.

New ABC sage.structure.element.NumberFieldElement, deprecate is_NumberFieldElement mkoeppe/sage
1 participant
@mkoeppe