Better partial support

[mwildehahn]

I noticed that when using Partial[MyModel] the model wasn't always populating required fields. With this change, the JSON schema we send to the model doesn't change (aside from having the model names updated to Partial{...} to make it match validation).

The naming of all the Partial / _Partial classes is a little weird. I was trying to keep this backwards compatible with the existing implementation. Open to suggestions on how to make this better.

---

	🚀 This PR description was created by Ellipsis for commit 6104107.

Summary:

This PR improves the handling of partial models in the instructor package, adds tests for the updated functionality, and updates some dependencies to optional in poetry.lock.

Key points:

• Updated Partial class in instructor/dsl/partial.py to better handle partial models.
• Added a new test file tests/dsl/test_partial.py to test the updated Partial class.
• Updated some dependencies to optional in poetry.lock.

---

Generated with ❤️ by ellipsis.dev

[ellipsis-dev]

👍 Looks good to me!

• Reviewed the entire pull request up to 6104107
• Looked at 385 lines of code in 3 files
• Took 1 minute and 57 seconds to review

More info

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

1. instructor/dsl/partial.py:98:

• Assessed confidence : 50%
• Comment:
  Ensure that the get_partial_model method is pure (i.e., it always returns the same result given the same arguments and doesn't have any side effects). This is important because the lru_cache decorator is used, which caches the results of the method calls with the given arguments.
• Reasoning:
  The lru_cache decorator is used on the get_partial_model method in the PartialBase class. This is a good practice as it will cache the results of the method calls with the given arguments, improving performance for repeated calls with the same arguments. However, it's important to ensure that the method is pure (i.e., it always returns the same result given the same arguments and doesn't have any side effects), otherwise the caching could lead to incorrect results.

2. instructor/dsl/partial.py:299:

• Assessed confidence : 50%
• Comment:
  Ensure that the change in the __class_getitem__ method of the Partial class, where the fields are wrapped with Partial but not made optional, doesn't break any existing functionality that relies on the fields being optional.
• Reasoning:
  The __class_getitem__ method in the Partial class is wrapping the fields with Partial but not making them optional. This is a change from the previous implementation where the fields were made optional. This change is explained in the comments and seems to be intentional, but it's important to ensure that this doesn't break any existing functionality that relies on the fields being optional.

3. tests/dsl/test_partial.py:16:

• Assessed confidence : 100%
• Grade: 80%
• Comment:
  Consider adding a test case that validates the functionality of the partial model with some partial data. This will ensure that the partial model can handle and validate partial data correctly.
• Reasoning:
  The test case test_partial is checking the JSON schema of the partial model. However, it's not actually testing the functionality of the partial model, i.e., whether it can handle partial data correctly. It would be beneficial to add a test case that validates the functionality of the partial model with some partial data.

Workflow ID: wflow_0wk46lx0qcXuolEe

---

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

[jxnl]

this is great yeah the _Partial might need some work but this looks correct.
don't have time to give a deep review but why does it need both?

if we break backwards compat, can we keep only one PArtial

[mwildehahn]

I was able to make this cleaner with the use of another class MakeFieldsOptional. Users of the lib can just do Partial[Model] but internally when we actually want to convert nested fields to optional we can do Partial[Model, MakeFieldsOptional].

[shanktt]

Changes look good to me! Nice approach with the MakeFieldsOptional, that's really clean