Hi Metaflow team, I have the situation where I wou...
# dev-metaflow
h
Hi Metaflow team, I have the situation where I would like to Subclass a Flow and only change certain tasks e.g.
Copy code
from metaflow import FlowSpec, step

class BaseFlow(FlowSpec):
    @step
    def start(self):
        print("this is the start")
        self.next(self.step1)

    @step
    def step1(self):
        print("base step 1")
        self.next(self.end)

    @step
    def end(self):
        print("base step end.")


class SubFlow(BaseFlow):
    @step
    def step1(self):
        print("sub step 1")
        self.next(self.step2)

if __name__ == "__main__":
    SubFlow()
Currently this is not supported as the graph itself is built by traversing the ast of the SubFlow class. I searched the issues on Github and couldn’t see this but has anyone worked on enabling the behaviour above?
I tested a small change locally where instead of using the ast nodevisitor to collect the nodes when building the graph I iterate on the dir(flow) so that steps from parent classes are included.
I raised a PR for this for discussion but happy to close this and discuss via an issue if this is preferred https://github.com/Netflix/metaflow/pull/2035
Just to give a tiny bit more context on the above PR. At my company we have a set of very similar flows to train various Machine Learning models. These have common steps such as data pulls, fitting, evaluating, saving artifacts etc. Currently we duplicate a lot of code from one flow to the next and the above change would allow us to inherit from a base training flow and overwrite perhaps just one step, for example changing some element of feature engineering.
Do I need to do anything else on the PR to help with review? I appreciate you may just be busy and will get to it when you have time.
d
sorry for the delay. I’ll try to take a look soon. The change looks pretty minor and is actually something we have been exploring (in a slightly different way — ie: to allow a configurable injection/removal of steps). So this may be good timing.
h
Hey Romain, thanks for the reply - no rush from my side. I actually moved this PR to be based off a feature branch in my fork https://github.com/Netflix/metaflow/pull/2086 rather than main
Hey Romain, I saw some messages on the PR relating to some netflix bot checks. Let me know if I can help at all 😃
d
I need to clean up some tests and will update shortly. Some tests broke internally and haven’t had time to fix them yet.
h
Ok, thanks Romain.
Hey Romain, is there anything I can help with on this PR?
d
I’m actually in the process of testing it out with our infra (but fixing tests 🙂). Thanks for the reminder and apologies for the delay. I am going to try to merge this today.
h
Thanks Romain! I hope it didn’t impact too many tests.
d
Oh. It wasn’t you. It was other stuff :). Just taking me a bit of time to get it all straightened out:).
h
Oh great, that’s relieving
d
@high-scientist-77658 — could you rebase your changes. I think I can now test easily
or I can create a dup and test it and then merge yours (want to get attribution right)
h
Thanks Romain, I have merged the latest master branch into the PR and will have a look into the issue you highlighted with
func_lineno
I have come back to you on the original PR. Thanks for reviewing!
d
awesome I’ll take a look and play around!
h
Hey Romain, the PR is ready for another review with the additional source file logging as discussed when you get time.
d
I haven’t forgotten this. It’s on my list. 2086 right?
h
Thanks, that’s the one. 😃
Here is the link for reference: https://github.com/Netflix/metaflow/pull/2086
Hey Romain, no rush on this but I was wondering when do you think you will get to reviewing this?
d
Hey. Apologies for the delay. It is absolutely on my list and I know it looks like a tiny change and it is but I need to verify it works witb some of the internal stuff we have and I haven’t had the time to get to it.
h
No problem Romain, I added the file source information to the logging as discussed in the PR. Let me know if I can help with any checks reuired.
d
I finished configs (finally) which was the big thing I needed to push out so now will do this over the break hopefully
🙌 1
h
Congrats on getting that finished!