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

add aqua tests #332

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

add aqua tests #332

wants to merge 6 commits into from

Conversation

lxvm
Copy link
Contributor

@lxvm lxvm commented Feb 12, 2024

As discussed in #328, I am opening a pr to address issues raised by Aqua.jl that could go into a breaking release of Polyhedra.jl. All of the changes are made are open to discussion, and I summarize them here:

  • Method ambiguities: There are many ambiguities with specific types from other packages from linear algebra functions, for example, so I focused on removing ambiguities within Polyhedra such as removevredundancy, rmap, rreps and sumpoints. A few ambiguities on * remain and I chose to ignore them
  • Unbound type parameters: hreps, points, hyperplanes, lines, rays, and halfspaces could only enforce a parametrized coefficient type if they all accept at least one representation. Instead of adding an extra argument to all of the functions I used promote to deduce the coefficient type. The Mesh type had an uninitialized parameter that I initialized to an empty array by default.
  • Undefined functions: A collection of functions prefixed by done*, start* and next* were never defined, so I removed them
  • Type piracy: I separated the overloaded type/Function FullDim into a FullDim type and typed_fulldim function. An alternative is to turn FullDim into a struct, like StaticArrays.Size, but I found that more difficult to implement since it requires more than search+replace. Most likely this will break implementations of the Polyhedra.jl interface, e.g. CDDLib.jl.

lxvm and others added 6 commits February 1, 2024 14:15
* move jump to extension

* support for regular dependency in julia 1.6

* reorganize JuMP ext

* move RecipesBase and GeometryBasics to ext

* uncomment verbose statement

* add error message for Mesh

* add news file describing breaking change

* update docs

* update examples

* simplify doc

* fix verbose print of model

* remove exported JuMP function

* import functions missing in GeometryBasicsExt

* add missing compat entry
* move jump to extension

* support for regular dependency in julia 1.6

* reorganize JuMP ext

* move RecipesBase and GeometryBasics to ext

* uncomment verbose statement

* add error message for Mesh

* add news file describing breaking change

* update docs

* update examples

* simplify doc

* fix verbose print of model

* remove exported JuMP function

* import functions missing in GeometryBasicsExt

* add missing compat entry
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (f582320) 88.92% compared to head (6332a57) 88.81%.

Files Patch % Lines
src/iterators.jl 86.66% 2 Missing ⚠️
src/redundancy.jl 66.66% 2 Missing ⚠️
src/representation.jl 60.00% 2 Missing ⚠️
src/aff.jl 87.50% 1 Missing ⚠️
src/liftedrep.jl 50.00% 1 Missing ⚠️
src/repop.jl 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
- Coverage   88.92%   88.81%   -0.11%     
==========================================
  Files          37       37              
  Lines        3170     3175       +5     
==========================================
+ Hits         2819     2820       +1     
- Misses        351      355       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

1 participant