FluxML / MLJFlux.jl

Wrapping deep learning models from the package Flux.jl for use in the MLJ.jl toolbox
http://fluxml.ai/MLJFlux.jl/
MIT License
143 stars 17 forks source link

Rethrow the error caught when builder fails #238

Closed ablaom closed 10 months ago

ablaom commented 10 months ago

Closes #237

There is presently a throw-catch block to check a user-provided builder will successfully create a Flux model. This PR ensures that the exception causing the fail is ultimately thrown.

codecov-commenter commented 10 months ago

Codecov Report

Patch coverage has no change and project coverage change: -0.61% :warning:

Comparison is base (4fa632d) 92.06% compared to head (4e4d27a) 91.45%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #238 +/- ## ========================================== - Coverage 92.06% 91.45% -0.61% ========================================== Files 12 12 Lines 315 316 +1 ========================================== - Hits 290 289 -1 - Misses 25 27 +2 ``` | [Files Changed](https://app.codecov.io/gh/FluxML/MLJFlux.jl/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Flux) | Coverage Δ | | |---|---|---| | [src/mlj\_model\_interface.jl](https://app.codecov.io/gh/FluxML/MLJFlux.jl/pull/238?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Flux#diff-c3JjL21sal9tb2RlbF9pbnRlcmZhY2Uuamw=) | `91.42% <0.00%> (-2.78%)` | :arrow_down: |

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

ablaom commented 10 months ago

@OkonSamuel Could you please review this? I've sent you an contributor invitation.

ablaom commented 10 months ago

I've added a test.

ablaom commented 10 months ago

I think we can ignore the nightly fail.

@OkonSamuel Are you happy for a merge?