Skip to content
This repository has been archived by the owner on Jul 3, 2023. It is now read-only.

Adds ability to pass in a callable to get the function name from for … #283

Merged
merged 4 commits into from
Jan 25, 2023

Conversation

skrawcz
Copy link
Collaborator

@skrawcz skrawcz commented Jan 23, 2023

…execute

I found myself wanting to click directly to outputs being created to inspect them. So to short cut that, directly referencing them is a way I can do that and have my IDE jump there when navigating code.

The downside is that this solution doesn't handle functions annotated with decorators that create nodes, e.g. @parameterize*, or @config* . I think that's okay, because it's probably clearer to have a "string" name here to indicate that something is changing/being produced at DAG creation time, and thus a reference to an exact function is likely a misnomer...

Adds unit test for this.

Example usage:

import data_loaders, transforms, model_pipeline
...
dr = driver.Driver(config, data_loaders, transforms, model_pipeline)  
dr.visualize_execution([model_pipeline.kaggle_submission_df], "./kaggle_submission_df.dot.png", {})
kaggle_submission_df: pd.DataFrame = dr.execute([model_pipeline.kaggle_submission_df])

The list can be a union of strings or callables. We right now just grab the name of the callable.

Changes

  • execute can now take in a "callable" that we take the name from.
  • adds unit test

How I tested this

  • locally
  • via unit tests

Notes

  • this will also make it simpler for IDEs to find references and potentially rename things when function names change.

Checklist

  • PR has an informative and human-readable title (this will be pulled into the release notes)
  • Changes are limited to a single goal (no scope creep)
  • Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future TODOs are captured in comments
  • Project documentation has been updated if adding/changing functionality.

…execute

I found myself wanting to click directly to outputs being created to inspect them.
So to short cut that, directly referencing them is a way I can do that and have
my IDE jump there when navigating code.

The downside is that this solution doesn't handle functions annotated with decorators
that create nodes, e.g. @parameterize*, or @config* . I think that's okay, because
it's probably clearer to have a "string" name here to indicate that something is
changing/being produced at DAG creation time, and thus a reference to an
exact function is likely a misnomer...

Adds unit test for this.
So that way you can't use a reference to a function to a module
that is not used. This should stop cases of copy paste erroring/changing
when people change something in the referenced function.
I think this might be the better default going forward? Let's try it
out and see.

I also replaced the module import, by directly importing the function
module and commenting that out. I had received feedback that it looked
complicated, but I just wanted to show you can script the import of modules,
so the driver script code could be argument driven...
Copy link
Collaborator

@elijahbenizzy elijahbenizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚢 🇮🇹

hamilton/driver.py Show resolved Hide resolved
hamilton/driver.py Show resolved Hide resolved
@skrawcz skrawcz merged commit 07607a2 into main Jan 25, 2023
@skrawcz skrawcz deleted the allow_function_pointers_to_execute branch January 25, 2023 06:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants