Nemocas / Nemo.jl

Julia bindings for various mathematical libraries (including flint2)
http://nemocas.github.io/Nemo.jl/
Other
176 stars 57 forks source link

Make dim check in matrix space creation consistent #1729

Closed lgoettgens closed 2 months ago

lgoettgens commented 2 months ago

Alternative to and thus closes https://github.com/Nemocas/Nemo.jl/pull/1711.

All matrix space methods now check r and c in the constructor of the type.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.93%. Comparing base (22e3908) to head (a837dca).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1729 +/- ## ========================================== - Coverage 84.93% 84.93% -0.01% ========================================== Files 95 95 Lines 37235 37237 +2 ========================================== - Hits 31627 31626 -1 - Misses 5608 5611 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

thofma commented 2 months ago

Hm, alternatively one could put them all into the user facing matrix_space functions, right?

lgoettgens commented 2 months ago

Hm, alternatively one could put them all into the user facing matrix_space functions, right?

Yes, it was basically a coin flip what I did

JohnAAbbott commented 2 months ago

Will this be merged soon? I have just ripped out my code (related to #1711), so that I could make PR #1732

JohnAAbbott commented 2 months ago

Just not to lose this observation: it is strange that the ccalls to flint pass the numbers of rows/cols as Int rather than UInt. No doubt there is a reason for this...

thofma commented 2 months ago

Can you explain why you think this is strange?

JohnAAbbott commented 2 months ago

Since the numbers of rows/cols must be non-negative and there is a type for non-negative values, UInt, it seems strange to use a type Int which also allows negative values (which should only lead to errors). I have not looked at the Flint sources...

thofma commented 2 months ago

The ccall arguments must match the signature of the C function.

JohnAAbbott commented 2 months ago

Ah, so my question was really about the flint UI; in C/C++ I understand one being wary of unsigned values. Thanks for merging!