MLJModelInterface.jl icon indicating copy to clipboard operation
MLJModelInterface.jl copied to clipboard

fix method ambiguity with Base.in in julia nightly Version 1.12.0-DEV…514

Open OkonSamuel opened this issue 1 year ago • 1 comments

This is needed to fix some failing test in another package. See https://github.com/JuliaAI/FeatureSelection.jl/actions/runs/9059544029/job/24887362493?pr=5#step:6:523

OkonSamuel avatar May 19 '24 20:05 OkonSamuel

I'm happy to approve this PR after appropriate testing (see below). However, it's is essentially a patch on top of a hack, for which I am personally to blame. That said, I think there is a fair chance that the overloading of in for MLJType (which is at least 3 years old, according to the blame) was rendered redundant when I refactored the learning networks code. The only way to find out for sure, would be to remove it and locally run

  • MLJBase tests
  • MLJTuning tests
  • MLJ#dev tests, with MLJ_TEST_INTEGRATION="true".

Is this something you would consider doing? Even if we accept the current PR, we probably want to run those tests anyway, so it's the same work really.

ablaom avatar May 19 '24 22:05 ablaom

Update: Investigations show that the replace method in MLJBase actually does use the overloading of Base.in in MLJModelInterface, and in a new PR I will implement the suggestion here for addressing the posted issue.

ablaom avatar Jul 19 '25 09:07 ablaom

Closing as rendered redundant by #220

ablaom avatar Jul 19 '25 11:07 ablaom