JuliaAI / MLJModels.jl

Home of the MLJ model registry and tools for model queries and mode code loading
MIT License
78 stars 27 forks source link

standardize AbstractFloats #555

Closed tiemvanderdeure closed 2 months ago

tiemvanderdeure commented 2 months ago

Changes the float type of the fitresult of Standardizer to AbstractFloat. This means standardizers will no longer force floats to Float64.

I also removed the second definition of fitresult_given_feature - I think this must have been an oversight.

ablaom commented 2 months ago

Thanks for this. It seems that codecov is hanging. I've been having this issue elsewhere. Can you try updating the codecov GH action to version @v3. You can do that here: https://github.com/JuliaAI/MLJModels.jl/blob/0de3aec7063f49a56d8f23252a42a4af896c599e/.github/workflows/ci.yml#L45

Don't use @v4 as that requires us to add a security token.

codecov[bot] commented 2 months ago

Codecov Report

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

Project coverage is 77.08%. Comparing base (5dc7eb2) to head (1757802). Report is 5 commits behind head on dev.

:exclamation: Current head 1757802 differs from pull request most recent head 66303c4. Consider uploading reports for the commit 66303c4 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #555 +/- ## ========================================== + Coverage 76.92% 77.08% +0.16% ========================================== Files 16 16 Lines 1170 1126 -44 ========================================== - Hits 900 868 -32 + Misses 270 258 -12 ```

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

ablaom commented 2 months ago

Could you please also fix this line:

https://github.com/JuliaAI/MLJModels.jl/blob/17578022018f08d31835a13021c52ee930605455/src/builtins/Transformers.jl#L506

More correct is eps(T), right?

tiemvanderdeure commented 2 months ago

https://github.com/JuliaAI/MLJModels.jl/blob/17578022018f08d31835a13021c52ee930605455/src/builtins/Transformers.jl#L506

More correct is eps(T), right?

eps(T) would fail if T is an integer, but eps(std(v)) should always work.