Closed fkuehlein closed 6 months ago
Attention: 10 lines
in your changes are missing coverage. Please review.
Comparison is base (
f374ffd
) 58.42% compared to head (601af09
) 60.72%.
Files | Patch % | Lines |
---|---|---|
src/pyunicorn/timeseries/recurrence_plot.py | 88.33% | 7 Missing :warning: |
src/pyunicorn/timeseries/cross_recurrence_plot.py | 50.00% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@fkuehlein, thank you for implementing and testing the required changes! Could you please carefully review my updates above before we merge? I found some minor remaining bugs, extended your tests to the "sparse" RQA use case, and refactored the Cython implementation to significantly reduce its redundancy. Hopefully, going forward, this will also make it easier to extend RQA methods and other highly parametric functions.
Cool, thank you @ntfrgl! Great to lose some weigth in that Cython module especially, should be helpful for future enhancements indeed. It all looks good to me.
test_recurrence_plot.py
as of a6f043d, trying not do overdo the parametrization-thing, as is hard to combine with comparison of numerical results without making everything poorly readable. Parametrization of fixtures may be cut even further as edge cases are covered by additional tests, but computational cost is small so I thought why not keep them.See #166 for some notes on previous commits.