Neuraxle icon indicating copy to clipboard operation
Neuraxle copied to clipboard

Feature: Create something like a _GhostHigherOrderStepMixin or something like a WrapperToHook step

Open guillaume-chevalier opened this issue 4 years ago • 4 comments

Is your feature request related to a problem? Please describe. The _HasChildrenMixin is interesting. MetaStepMixin as well, and MetaStep, and TruncableStep. However, these steps implies pushing to the context and deepends not only the stack trace, but also the subfolders upon saving, and the flat hyperparameter naming addresses in the apply methods. What if a wrapper could be invisible to all and have its named skipped in the tree ? E.g.: assertion wrappers shouldn't make the subfolders nested deeper with another name for the assertion wrapper. Same for some other kind of wrappers out there.

Describe the solution you'd like class _GhostHigherOrderStepMixin(_HasChildrenMixin) that would not affect pathing.

Describe alternatives you've considered No other alternatives imagined. However, I fear that doing this will mess up with the savers' paths and won't allow for properly saving and reloading those ghost higher order steps.

Additional context Neuraxle Steps are like UI/Frontend frameworks' concepts of Components. Read more here:

  • https://reactjs.org/docs/higher-order-components.html

So "higher order" means "wrapper" in our language, and "step" is like "component".

guillaume-chevalier avatar Dec 18 '20 19:12 guillaume-chevalier

As for alternatives we could have the service assertion wrappers be step instead. We'd then either loose the step.assert_has_service() syntactic sugar or have that function return the two steps and have pipelines instance to automatically unfold its elements tuple elements into sequential steps.

This alternative seems way less elegant and less versatile than what you've proposed, but It wouldn't be very hard to implement.

vincent-antaki avatar Dec 20 '20 23:12 vincent-antaki

I see a possible way out of this: editing the MetaStep and TruncableStep 's savers so as to save the higher order step that they contain, but within themselves. This means that the higher order steps would have no special state, no saver of their own, and would be saved by the default saver of their parents (that necessarily are one of a: MetaStep, TruncableStep, or a root). This allows for not having to name them in the saving tree, and it also allows for these steps to not have any apply functions, nor hyperparams, nor anything too funky. So the ghost higher order steps would be very very light, not even a BaseTransformer. It would inherit only some of the core mixins.

However, as a downside, it complexifies the saving of the MetaStep, TruncableStep, or nothingness (root) that contains them. One solution for the root thing is to always wrap everything that is saved within an IdentityWrapper when .save is called, at the root level.

@vincent-antaki

guillaume-chevalier avatar Feb 21 '21 07:02 guillaume-chevalier

Seems totally doable. I wonder if, instead of adapting MetaStep and TruncableStep's savers, we shouldnt define a GhostMetaStep and GhostTruncableStep. This would allow us to override the constructor and thus simply insert a different saver than the default one. It sounds like a simpler solution, albeit not very clean code abiding.

vincent-antaki avatar Feb 22 '21 05:02 vincent-antaki

Simple solution found: if doing hooks, such a ghost step could just return a hooked version of its wrapped step and it would act just like a modifier to a step instead of a complete wrap:

  • #320

We could perhaps edit some of the current wrappers to be such hooks to avoid having some crazy subfolder paths in the cache folders.

Idea: even have a class like WrapperToHook(wrapper, wrapped) that would take a wrapper, decapitulate it into some hookables, and apply the hookables on the wrapped step to return a modified (hooked) wrapped step. Precompute some Pre-Hooked wrappers with partial methods to the above WrapperToHook so as to import them ready for usage as ghost wrappers that just hook.

guillaume-chevalier avatar Oct 21 '21 15:10 guillaume-chevalier

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs in the next 180 days. Thank you for your contributions.

stale[bot] avatar Apr 24 '23 18:04 stale[bot]