Added the ability to override nodes from later imported modules: https://github.com/DAGWorks-Inc/hamilton/pull/1134 The only part I am unsure about is how to update docs.

Thanks for the contribution! I see the thread of issues and Slack convo. I think it's worth discussing the feature a bit more before you commit additional time to it! For one, relying on ordering of arguments can be brittle. For example

.with_modules(foo, bar, allow_override=True)

might not equal

.with_modules(bar, foo, allow_override=True)

. It's good to use the flag

allow_override

to prevent users accidentally overriding functions though Since this change involves many core internals (

FunctionGraph

,

Driver

,

Builder

) I'm wondering if there are other ways to support this feature. Hamilton couples a lot of thing to the

module

Python construct (for better or worse). But concretely, the issue seems to be overriding functions. It might be a valuable change to catch duplicate functions "closer to the user" (e.g.,

Builder.build()

rather than in internal components like

FunctionGraph

Hi, thanks for checking over. Can you expand on your thoughts a bit, the way I understand you now is that you want to refactor

find_functions()

all the way from

graph.py

and move it to the

Builder

? Or do you propose to double check and pre-prepare the functions before they get passed to the

create_function_graph

? There is another entry point by creating the

Driver

directly without using the

Builder

- not sure how many people use it this way, but one of the first examples on the website does that. I don't think we can only push it onto the task of the

Builder

.

An idea would be to use

find_functions()

, which is a public-facing API part of

hamilton.graph_utils

. Ideally, it would allow us to only make "surface level" changes closer to the

Builder

. ## Default behavior We can keep

.with_modules()

as-is and avoid changes to other internals (

Driver

,

FunctionGraph

). This ensures safety for all Hamilton users ## Override behavior When specifying

.with_modules(..., allow_overrides=True)

, the logic of

.with_modules()

could directly call

find_functions()

and dynamically removed duplicate functions

def with_modules(*modules):
   if allow_overrides:
      functions_seen = set()
      for module in modules:
         functions = graph_utils.find_functions(module)
         for name, callabl in functions:
            if name in functions_seen
               del getattr(module, name)
            else:
               functions_seen.add(name)
   
    self.modules.extend(modules)

@Jernj Frank — really appreciate the contribution! I think this makes sense to use the internal API, and I’m not particularly concerned about relying on ordering. This is already a bit of a power-mode feature, so people will be OK with the caveats of ordering. Re the

allow_overrides

flag — we specifically decided to put it centrally, rather than on the

with_modules

due to the fact that it gets you to odd behavior if it’s on an individual module call. E.G. what if you add it in one with_modules and not the other? More stuff to think about. We also can’t catch it on the builder @Thierry Jean — it has no idea what the nodes are, as we construct the fnction graph. We could always rearchictect (construct the function graph beforehand then merge a bunch), but given the way it’s built, I think this is a very reasonable approach. Function graph is specifically repsonsible for taking all the functions and putting it together. Some nuance in how we handle it with modules/functions/who knows about what, but that’s all stuff that can change because it’s an internal API. The more we push out, the harder of a time we have doing that later.

Seems like it does a new build every time I push and updates the docs as well. Bit of a wait to get to the generate docs part, but better than needing to locally install all the things needed for generating docs

Yeah, locally installing is good if you do more than a few iterations (it’s super fast), but is a bit of work to set up