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

sage.matrix.operation_table: Modularization and code style fixes #35153

Merged
merged 3 commits into from
Mar 26, 2023

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Feb 17, 2023

πŸ“š Description

We remove the module-level dependency of sage.matrix.operation_table on matplotlib and sage.plot and fix some code style issues.

It fixes a modularization regression introduced in #8598.

πŸ“ Checklist

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

βŒ› Dependencies

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2023

Codecov Report

Base: 88.59% // Head: 88.58% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (fb4f6ab) compared to base (fbb4127).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #35153      +/-   ##
===========================================
- Coverage    88.59%   88.58%   -0.01%     
===========================================
  Files         2140     2140              
  Lines       396961   396958       -3     
===========================================
- Hits        351677   351656      -21     
- Misses       45284    45302      +18     
Impacted Files Coverage Ξ”
src/sage/matrix/operation_table.py 96.58% <100.00%> (+0.91%) ⬆️
src/sage/crypto/util.py 91.37% <0.00%> (-5.18%) ⬇️
src/sage/interfaces/qsieve.py 71.30% <0.00%> (-2.61%) ⬇️
...e/combinat/cluster_algebra_quiver/mutation_type.py 50.83% <0.00%> (-2.14%) ⬇️
src/sage/combinat/combination.py 93.50% <0.00%> (-1.50%) ⬇️
src/sage/modular/hecke/algebra.py 94.65% <0.00%> (-1.07%) ⬇️
src/sage/schemes/elliptic_curves/hom_velusqrt.py 94.09% <0.00%> (-0.79%) ⬇️
src/sage/geometry/cone.py 93.01% <0.00%> (-0.60%) ⬇️
src/sage/schemes/elliptic_curves/cardinality.py 87.00% <0.00%> (-0.40%) ⬇️
src/sage/quadratic_forms/ternary_qf.py 66.73% <0.00%> (-0.39%) ⬇️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

β˜” View full report at Codecov.
πŸ“’ Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Just a few trivial doc formatting things. LGTM otherwise.

src/sage/matrix/operation_table.py Outdated Show resolved Hide resolved
src/sage/matrix/operation_table.py Outdated Show resolved Hide resolved
src/sage/matrix/operation_table.py Outdated Show resolved Hide resolved
Co-authored-by: Travis Scrimshaw <clfrngrown@aol.com>
@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 18, 2023

Thank you! I've committed these changes.

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM.

@github-actions
Copy link

Documentation preview for this PR is ready! πŸŽ‰
Built with commit: fb4f6ab

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

lgtm.

@Bruno-TT - who wrote that graphic tables code - might want to have a look, but I guess it's all right.

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 18, 2023

Thanks, Dima!

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.

None yet

6 participants