JuliaDynamics / ComplexityMeasures.jl

Estimators for probabilities, entropies, and other complexity measures derived from data in the context of nonlinear dynamics and complex systems
MIT License
48 stars 11 forks source link

Deprecate passing `m` as keyword in ordinal pattern stuff #333

Closed Datseris closed 8 months ago

Datseris commented 8 months ago

Closes #309

codecov[bot] commented 8 months ago

Codecov Report

Merging #333 (caaabb9) into main (d8c2d80) will increase coverage by 0.16%. Report is 2 commits behind head on main. The diff coverage is 96.42%.

@@            Coverage Diff             @@
##             main     #333      +/-   ##
==========================================
+ Coverage   88.03%   88.19%   +0.16%     
==========================================
  Files          78       78              
  Lines        2031     2042      +11     
==========================================
+ Hits         1788     1801      +13     
+ Misses        243      241       -2     
Files Coverage Δ
src/deprecations.jl 100.00% <100.00%> (ø)
src/encoding_implementations/ordinal_pattern.jl 86.79% <100.00%> (-0.25%) :arrow_down:
src/outcome_spaces/ordinal_patterns.jl 95.69% <92.85%> (ø)

... and 1 file with indirect coverage changes

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Datseris commented 8 months ago
  • Why are all keyword arguments to the estimators now positional arguments instead? I tried just moving m as a type parameter in the examples, but then the constructors fail, because they don't accept keywords anyone.

Don't quote me on this, but I recall that type stability is propagated properly at optional positional arguments but not at keyword arguments, and here the function type for isless was a keywprd argument.

Datseris commented 8 months ago

Also note that I added deprecated versions of the functions in hte deprecations.jl file that use the keyword arguments. So that should work (fingers crossed)

Datseris commented 8 months ago

i'll add a test for the deprecations and then merge this?

kahaaga commented 8 months ago

i'll add a test for the deprecations and then merge this?

Yep!