Adds target_ parameter to save_to

[elijahbenizzy]

This allows us to layer materialization decorators. Previously it would use the output of the other node, and thus try to save the result of the other saver. This allows them both to specify.

Changes

How I tested this

Notes

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.

---

	🚀 This PR description was created by Ellipsis for commit 8ecadca.

Summary:

This PR enhances the SaveToDecorator class by adding a target_ parameter, enabling layered materialization decorators to specify which node to use, and includes a new test case for this functionality.

Key points:

• Enhanced the SaveToDecorator class by adding a target_ parameter.
• Enabled layered materialization decorators to specify which node to use.
• Added a new test case test_save_to_decorator_with_target in tests/function_modifiers/test_adapters.py.
• Updated create_saver_node, save_data functions and input_types dictionary to use node_to_save determined by target_.
• Corrected __call__ method in save_to class to return SaveToDecorator.

---

Generated with ❤️ by ellipsis.dev

[ellipsis-dev]

❌ Changes requested.

• Reviewed the entire pull request up to 8ee411e
• Looked at 67 lines of code in 1 files
• Took 1 minute and 8 seconds to review

More info

• Skipped 0 files when reviewing.
• Skipped posting 0 additional comments because they didn't meet confidence threshold of 50%.

Workflow ID: wflow_wOvUfJuODYLPk1kt

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.

[skrawcz]

unit test?

[elijahbenizzy]

unit test?

Updated, had forgot to push

[ellipsis-dev]

👍 Looks good to me!

• Performed an incremental review on 7a1fe8f
• Looked at 135 lines of code in 2 files
• Took 1 minute and 32 seconds to review

More info

• Skipped 0 files when reviewing.
• Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.

1. hamilton/function_modifiers/adapters.py:495:

• Assessed confidence : 10%
• Comment:
  The addition of the target_ parameter to the SaveToDecorator class is well implemented and allows for more flexibility when using the decorator. The changes are logically sound and the code is clean. Good job on including a test case for this new functionality.
• Reasoning:
  The PR adds a new parameter target_ to the SaveToDecorator class. This parameter allows the decorator to specify the target node for saving data. This is particularly useful when layering materialization decorators. The changes seem to be correctly implemented and the logic makes sense. The code is clean and follows the DRY principle. The PR also includes a test case for the new functionality, which is a good practice.

Workflow ID: wflow_17wbD1CJ0QifUsmr

---

Not what you expected? You can customize the content of the reviews using rules. Learn more here.

[skrawcz]

Looks good. But seems like docs are failing.

[ellipsis-dev]

👍 Looks good to me!

• Performed an incremental review on 5af4d54
• Looked at 13 lines of code in 1 files
• Took 2 minutes and 14 seconds to review

More info

• Skipped 0 files when reviewing.
• Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.

1. hamilton/function_modifiers/adapters.py:659:

• Assessed confidence : 0%
• Comment:
  The addition of the target_ parameter to the SaveToDecorator class is a good enhancement. It allows specifying the node to save when layering savers, which increases the flexibility of the decorator. The changes in the create_saver_node method correctly use this new parameter. Good job!
• Reasoning:
  The PR adds a new parameter target_ to the SaveToDecorator class. This parameter is used to specify the node to save when layering savers. The code changes look correct and follow the best practices. The new parameter is used correctly in the create_saver_node method. The PR also includes a new test case for this functionality.

Workflow ID: wflow_DGZX8LVPJwOt9gxR

---

Not what you expected? You can customize the content of the reviews using rules. Learn more here.

[ellipsis-dev]

👍 Looks good to me!

• Performed an incremental review on 8ecadca
• Looked at 135 lines of code in 2 files
• Took 2 minutes and 58 seconds to review

More info

• Skipped 0 files when reviewing.
• Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.

1. hamilton/function_modifiers/adapters.py:495:

• Assessed confidence : 10%
• Comment:
  The changes in this PR appear to be correctly implemented. The new target_ parameter in the SaveToDecorator class is used correctly to determine the node_to_save in the create_saver_node and save_data methods. The new test case test_save_to_decorator_with_target should adequately test the new functionality.
• Reasoning:
  The PR adds a new parameter target_ to the SaveToDecorator class. This parameter allows the user to specify which node to use when layering materialization decorators. The changes seem to be implemented correctly, with the target_ parameter being used to determine the node_to_save in the create_saver_node and save_data methods. The PR also includes a new test case test_save_to_decorator_with_target to verify the new functionality. The test case seems to be correctly implemented and should adequately test the new functionality. Overall, the changes in this PR appear to be correctly implemented and should not introduce any new issues.

Workflow ID: wflow_FVH4Ax7NWmEJBhu9

---

Not what you expected? You can customize the content of the reviews using rules. Learn more here.