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

Improve code style for matrix2.pyx file #36877

Merged
merged 11 commits into from
Jan 14, 2024

Conversation

RuchitJagodara
Copy link
Contributor

  • Correct pycodestyle and remove check condition for columns in prod_of_row_sums() function
  • Improve code style and remove an unwanted if condition in restrict() function
  • Improve code style of matrix2.pyx file
  • Improve code style for matrix2.pyx

This patch improves the code style as mentioned in the sage documentation and removes
some unnecessary lines which were commented before and was not removed.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.

⌛ Dependencies

None

Corrected code style according to sagemath standards.
…_row_sums() function

This commit removes check condition for column number that whether
the column number is not exceeding the index but this is done somewhere else
and no longer needed. And this corrects pycodestyle of file matrix2.pyx
…function

This commit improves code style further and removes a if condition
in restrict() function.
Improves code style of the file matrix2.pyx
Improving code style for the file matrix2.pyx
Improved the code style for matrix2.pyx file.
Final commit for the improved code style for matrix2.pyx file.
@fchapoton
Copy link
Contributor

Adding the missing whitespace after commas is much too intrusive to be done systematically, and in particular in huge files such as the one touched here. To say that otherwise, we are not doing that, except just one in passing when modifying the code for other reasons.

@RuchitJagodara
Copy link
Contributor Author

Hmm.. got it. Actually, I was modifying the code of this file when I saw this, so I thought it would be better to correct that.

@RuchitJagodara
Copy link
Contributor Author

@fchapoton, can you please help me find out what the problem is with the build and test check?

Copy link

github-actions bot commented Jan 1, 2024

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

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

LGTM.

@kwankyu
Copy link
Collaborator

kwankyu commented Jan 4, 2024

@fchapoton, can you please help me find out what the problem is with the build and test check?

There is no problem. Ignore conda tests.

@RuchitJagodara
Copy link
Contributor Author

Ok, thank you !

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 5, 2024
    
- Correct pycodestyle and remove check condition for columns in
prod_of_row_sums() function
- Improve code style and remove an unwanted if condition in restrict()
function
- Improve code style of matrix2.pyx file
- Improve code style for matrix2.pyx

This patch improves the code style as mentioned in the sage
documentation and removes
some unnecessary lines which were commented before and was not removed.
<!-- ^^^^^
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.
- [x] The description explains in detail what this PR is about.

### ⌛ 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! -->
None
    
URL: sagemath#36877
Reported by: Ruchit Jagodara
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit fe174bf into sagemath:develop Jan 14, 2024
14 of 20 checks passed
@mkoeppe mkoeppe added this to the sage-10.3 milestone Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants