automl / amltk

A build-it-yourself AutoML Framework
https://automl.github.io/amltk/
BSD 3-Clause "New" or "Revised" License
68 stars 6 forks source link

[JOSS Review] Pipeline example not working #269

Open gomezzz opened 8 months ago

gomezzz commented 8 months ago

The minimal example on pipelines is not working for me.

(opened as part of JOSS Review https://github.com/openjournals/joss-reviews/issues/6367 )

eddiebergman commented 8 months ago

I fixed the example, many thanks. I'm surprised this was broken. Unfortunatly the README is the few places we can't automatically run code samples. The main documentation's code is tested in CI to ensure it runs correctly :)

The issue was actually a stray , (comma), giving back two objects.

The issue with dask was a bad import that came from the wrong place.

I've fixed both in this PR so many thanks for finding these, hopefully you can copy and paste the example (with sklearn installed).


As a side note, the dask parts of AMLTK are all optional which is a re-occuring pattern for quite a few integrations. This is not the first time an import accidentally escaped. Would you have any suggestions on ensuring this with CI?

gomezzz commented 8 months ago

Hi @eddiebergman ,

trying to run the example from #270 I now receive

  .../envs/amltk/lib/python3.11/site-packages/amltk/pipeline/parsers/configspace.py", line 146, in <module>
    from ConfigSpace import Categorical, ConfigurationSpace, Constant
ModuleNotFoundError: No module named 'ConfigSpace'
eddiebergman commented 8 months ago

Aye, that would need to be installed, as referenced in another issue, it's an entirely optional dependancy (but required with SMAC).

One option would be pip install ConfigSpace or pip install amltk[smac] which installs all the optional dependacies for the smac optimizer.

The other option would be to use pip install amltk[optuna] and use the Optuna optimizer instead.


I guess the main issue that needs to be addressed here is including a comment or something that mentions what dependancies are needed for which example.

gomezzz commented 8 months ago

Yes, works now with the deps :)