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

Fix E721 warnings for .pyx files #36036

Merged
merged 5 commits into from
Aug 13, 2023
Merged

Fix E721 warnings for .pyx files #36036

merged 5 commits into from
Aug 13, 2023

Conversation

jhpalmieri
Copy link
Member

@jhpalmieri jhpalmieri commented Aug 4, 2023

Fix E721 warnings for .pyx files

pycodestyle reports: "E721 do not compare types, for exact checks use is / is not, for instance checks use isinstance()." So replace type(x) == str with isinstance(x, str), replace type(x) != type(y) with type(x) is not type(y), etc. This is a followup to #36032 which dealt with .py files.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

pycodestyle reports: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`. So replace `type(x) == str` with `isinstance(x, str)`, replace `type(x) != type(y)` with `type(x) is not type(y)`, etc.
src/sage/rings/integer.pyx Outdated Show resolved Hide resolved
src/sage/rings/integer.pyx Outdated Show resolved Hide resolved
@jhpalmieri
Copy link
Member Author

Thank you for the suggestions; I've implemented them.

@jhpalmieri
Copy link
Member Author

I'm not sure how much of an effect "Resolve conversation" has, but it's fun to click buttons...

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 5, 2023

I'm not sure how much of an effect "Resolve conversation" has

It just collapses the comments for all readers. Good idea to do when a comment has been taken care of and you're not adding a response.

@jhpalmieri
Copy link
Member Author

Thanks! I will rebase on the next beta when it's released.

@jhpalmieri
Copy link
Member Author

This merges cleanly with 10.1.beta9 as is.

@vbraun
Copy link
Member

vbraun commented Aug 10, 2023

Doesn't build, but somehow has positive review

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2023

Indeed

[sagelib-10.1.beta8]     [7/9] Cythonizing sage/rings/integer.pyx
[sagelib-10.1.beta8]     [9/9] Cythonizing sage/structure/category_object.pyx
[sagelib-10.1.beta8] 
[sagelib-10.1.beta8]     Error compiling Cython file:
[sagelib-10.1.beta8]     ------------------------------------------------------------
[sagelib-10.1.beta8]     ...
[sagelib-10.1.beta8]             if isinstance(m, Integer):
[sagelib-10.1.beta8]                 elog = self.exact_log(m)
[sagelib-10.1.beta8]                 if elog == -sage.rings.infinity.infinity or m**elog == self:
[sagelib-10.1.beta8]                     return elog
[sagelib-10.1.beta8] 
[sagelib-10.1.beta8]             if (isinstance(m, Rational):
[sagelib-10.1.beta8]                                       ^
[sagelib-10.1.beta8]     ------------------------------------------------------------
[sagelib-10.1.beta8] 
[sagelib-10.1.beta8]     sage/rings/integer.pyx:2860:35: Expected ')', found ':'
[sagelib-10.1.beta8]     Traceback (most recent call last):

@github-actions
Copy link

Documentation preview for this PR (built with commit d57321a; changes) is ready! 🎉

@vbraun vbraun merged commit b268272 into sagemath:develop Aug 13, 2023
@mkoeppe mkoeppe added this to the sage-10.1 milestone Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants