Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace Python 2 'long' with Python 3 'int' #39091

Merged
merged 2 commits into from
Dec 15, 2024

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented Dec 8, 2024

After Cython commit cython/cython@d0f53f4 , long is no longer valid in .pyx file.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@user202729 user202729 force-pushed the remove-python2-long branch 2 times, most recently from bca8137 to 1d1fbdf Compare December 8, 2024 18:24
Comment on lines 583 to 584
if isinstance(x, (int, Integer)) \
or isinstance(x, rationallib.mpq):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(x, (int, Integer)) \
or isinstance(x, rationallib.mpq):
if isinstance(x, (int, Integer, rationallib.mpq)):

@@ -818,7 +818,7 @@ cdef class RealIntervalField_class(sage.rings.abc.RealIntervalField):
# Direct and efficient conversions
if S is ZZ or S is QQ:
return True
if S is int or S is long:
if S is int:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can join that with the previous conditional.

Comment on lines 625 to 626
if isinstance(x, (int, Integer)) \
or isinstance(x, rationallib.mpq):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(x, (int, Integer)) \
or isinstance(x, rationallib.mpq):
if isinstance(x, (int, Integer, rationallib.mpq)):

@@ -1358,7 +1358,7 @@ cdef class FiniteField(Field):
False
"""
from sage.rings.integer_ring import ZZ
if R is int or R is long or R is ZZ:
if R in int or R is ZZ:
Copy link
Contributor

@mantepse mantepse Dec 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if R in int or R is ZZ:
if R is int or R is ZZ:

if this didn't give a failed test, it might make sense to add a doctest. Well, maybe not.

Copy link

github-actions bot commented Dec 8, 2024

Documentation preview for this PR (built with commit f398a0f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Contributor

@mantepse mantepse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 9, 2024
After Cython commit cython/cython@d0f53f40f8c1
8da747919449febc2344ebb43e5f , `long` is no longer valid in `.pyx` file.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

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

URL: sagemath#39091
Reported by: user202729
Reviewer(s): Martin Rubey
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 11, 2024
    
After Cython commit cython/cython@d0f53f40f8c1
8da747919449febc2344ebb43e5f , `long` is no longer valid in `.pyx` file.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39091
Reported by: user202729
Reviewer(s): Martin Rubey
vbraun pushed a commit to vbraun/sage that referenced this pull request Dec 12, 2024
    
After Cython commit cython/cython@d0f53f40f8c1
8da747919449febc2344ebb43e5f , `long` is no longer valid in `.pyx` file.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39091
Reported by: user202729
Reviewer(s): Martin Rubey
@vbraun vbraun merged commit 2059f42 into sagemath:develop Dec 15, 2024
23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants