Add cli entrypoint for compile #218
Conversation
GeorgesLorre
requested review from
PhilippeMoussalli,
RobbeSneyders,
NielsRogge and
ChristiaensBert
fondant/cli.py
Outdated
| from fondant.pipeline import Pipeline | ||
|
|
||
|
|
||
| class PipelinePythonReference(argparse.Action): |
There was a problem hiding this comment.
I did not find anything standard on importstrings (file.py:my_object) so I made my own parser.
There was a problem hiding this comment.
Any reason to go for a custom Action instead of a custom type? I think a custom type could be a bit simpler.
FYI, reference implementation from uvicorn.
fondant/cli.py
Outdated
| """Attempt to load a pipeline from a module file path and object name.""" | ||
| spec = importlib.util.spec_from_file_location(object, module) | ||
| pipeline = importlib.util.module_from_spec(spec) # type: ignore | ||
| spec.loader.exec_module(pipeline) # type: ignore |
There was a problem hiding this comment.
Not that happy with this mess but works for now
There was a problem hiding this comment.
See my comment above for an implementation based on import_module and getattr.
GeorgesLorre
force-pushed
the
feature/local-starcoder
branch
from
6dd333e
to
30db0d8
Compare
GeorgesLorre
force-pushed
the
feature/fondant-cli
branch
from
7c29ed6
to
a8c1ec5
Compare
There was a problem hiding this comment.
Thanks @GeorgesLorre! I know it must hurt to see the coverage decline, but looking at the uncovered lines, I don't think there's a lot to test without just duplicating the same logic.
| @@ -1,5 +1,6 @@ | |||
| """Pipeline used to create the dataset to train the StarCoder model.""" | |||
|
|
|||
| import argparse | |||
There was a problem hiding this comment.
Why was this added? I think we should be able to remove the if __name__ = "__main__": block at the bottom of the file, no?
fondant/compiler.py
Outdated
| # add component spec to command if does not exist yet | ||
| if not component_op.arguments.get("component_spec"): | ||
| component_spec = component_op.component_spec.specification | ||
| command.extend(["--component_spec", json.dumps(component_spec)]) | ||
|
|
There was a problem hiding this comment.
You added this to the ComponentOp class in #215, right?
There was a problem hiding this comment.
This should be left out of git.
WIP! First implementation of the fondant CLI (tried and tested): `fondant compile -p /playground/mycoolpipeline.py:pipeline` (note you will need to (re)run `poetry install --all-extras`) I'm mainly looking for feedback on this approach before I spend more time expanding it
WIP!
First implementation of the fondant CLI (tried and tested):
fondant compile -p /playground/mycoolpipeline.py:pipeline(note you will need to (re)run
poetry install --all-extras)I'm mainly looking for feedback on this approach before I spend more time expanding it