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.geometry.integral_points: Use generic impl if no matrix_integer_dense #35135

Merged
merged 3 commits into from
Mar 26, 2023

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Feb 15, 2023

πŸ“š Description

This is for the distribution sagemath-polyhedra (#32432) so that it does not have to depend on the flint library.

πŸ“ 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

@mkoeppe mkoeppe requested a review from kliem February 15, 2023 02:14
@mkoeppe mkoeppe changed the title sage.geometry.integral_points: Use generic impl if no matrix_integer_dense sage.geometry.integral_points: Use generic impl if no matrix_integer_dense Feb 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Base: 88.59% // Head: 88.59% // Increases project coverage by +0.00% πŸŽ‰

Coverage data is based on head (9a8b269) compared to base (84eebf0).
Patch coverage: 60.00% of modified lines in pull request are covered.

❗ Current head 9a8b269 differs from pull request most recent head 60e3509. Consider uploading reports for the commit 60e3509 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #35135   +/-   ##
========================================
  Coverage    88.59%   88.59%           
========================================
  Files         2140     2141    +1     
  Lines       396961   396965    +4     
========================================
+ Hits        351671   351703   +32     
+ Misses       45290    45262   -28     
Impacted Files Coverage Ξ”
src/sage/schemes/toric/sheaf/klyachko.py 92.43% <ΓΈ> (ΓΈ)
src/sage/geometry/integral_points.py 50.00% <50.00%> (ΓΈ)
src/sage/matrix/matrix_space.py 89.60% <100.00%> (ΓΈ)
src/sage/homology/matrix_utils.py 84.40% <0.00%> (-3.67%) ⬇️
src/sage/combinat/combination.py 93.50% <0.00%> (-1.50%) ⬇️
src/sage/groups/generic.py 88.34% <0.00%> (-0.68%) ⬇️
src/sage/graphs/graph_plot.py 84.33% <0.00%> (-0.57%) ⬇️
src/sage/quadratic_forms/ternary_qf.py 66.73% <0.00%> (-0.39%) ⬇️
src/sage/modular/arithgroup/arithgroup_perm.py 92.57% <0.00%> (-0.38%) ⬇️
src/sage/groups/cactus_group.py 98.89% <0.00%> (-0.37%) ⬇️
... and 18 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.

@github-actions
Copy link

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

@@ -0,0 +1,6 @@
#cython: wraparound=False, boundscheck=False
Copy link
Contributor

@kliem kliem Feb 18, 2023

Choose a reason for hiding this comment

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

To my understanding, this file will just not build, if flint is not present.

From my quick "research" of the state of things, this file will not be included in a sage polyhedra distribution, if flint is not present?

Copy link
Member Author

Choose a reason for hiding this comment

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

Which modules are included in any distribution is static.
This module is simply not included in sagemath-polyhedra and will be provided by a different distribution.

@mkoeppe
Copy link
Member Author

mkoeppe commented Feb 18, 2023

Thanks for the review!

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

5 participants