Closed MothNik closed 2 months ago
This Pull request should be squashed before merging.
For the documentation, the only thing that changed is the API of solve.
I hope you like the additional details, although the type hint for solver
on top is a bit big.
Edit June 09, 2024
27 is an improved version of this pull request. The following describes the basic changes that #27 relies on. However, this pull request is not really required anymore. I will still leave it open for a bit.
This pull request primarily tackles #11 by enabling multiple right hand sides for the
pentapy
-solvers. Currently, this is only implemented in a serial fashion because the current API does not specify the memory layout of the input Arrays. This makes it quite hard to completely remove the GIL which is required for parallelization of Cython level, but the serial fashion implements the feature in a first step β Implementing this required a shotgun refactoring on Cython-level for a consistent, scalable flow and memory handling that relies on memoisation.Furthermore the pull request fixes
23 to acutally rely on the error handling that was never triggered,
24 via a fallback to NumPy.
25 by dropping the superficial
unittest
for an extensive, parallelizedpytest
test suite which can now be run byThis increased the coverage to 100%. It started out from 80% on a line-hit basis, but the test was not really testing the package. Now, many different combinations and scenarios are covered, so the coverage increase is way more than the visible 20%. Anyway, the coverage was pretty meaningless before because it was not really considered in the package development. As an example it uncovered #23 showing two massively red uncovered blocks at the first test that was run.
I've found the development with the current package setup causes so much friction on simple tasks and maintainability was not really facilitated, so
Enum
-conversion and only relying on single checks. Also, the string-input was made case-insensitive.pyproject.toml
. To install a new dependency for development,pip install -r some_requirements.txt
is way simpler thanpip install pentapy[...]
which takes very long for rebuildingpentapy
. Actually, I'm not even sure whether all the development requirements should be in thepyproject.toml
. Nobody who just installs the package will notice the difference, but for development, this ensures a proper workflow. NOTE: this was done in a single commit, so it can be reverted in one go if desired.matrix has to be n x n
is nowmatrix has to be n x n, got 10 x 15
, so the user knows where to check.pentapy.solve
is now fully typed, especially thesolver
-argument which can be specified by so many literals that leaving it untyped reduces the joy to work with it. Now, any IDE will show all the options as one hovers over the function name and also autocompletion with proper input is now easily possible.Finally, some typos were fixed. With a VSCode extension like CSpell or a similar spell checker, this can be avioded.
I tried to update the changelog, but feel free to adapt this. I don't really know what has to be updated in terms of other documentations and stuff around, but you know better. I also don't know where to update the version and if these changes are a minor or a major jump for the version.
I really hope that I didn't miss anything, but all the new tests pass and the bound checker never ran into an out of bounds error β
Sorry, that this is such a big PR, but I would not have been able to ensure that everything works fine with the current package setup π