Skip to content

Conversation

NeejWeej
Copy link
Collaborator

@NeejWeej NeejWeej commented Oct 2, 2025

Add PathKeyResolverMixin which allows specifying some key/value pairs from python code via a PyObjectPath. This allows offloading some of the configuration to python, getting benefits from:
easier testability, cohesiveness of related configurations, and deduplication.

One specific workflow this helps with, is that partial functions can be constructed by taking a general CallableModel, and specializing (specifying specific/partial configuration options) it for various use-cases. The default resolved_wins, provides a (overrideable) check that pre-defined configurations in the code are not accidentally overwritten, and reduces the surface area exposed that could be overriden via yaml.

This functionality is opt-in, since base ccflow.BaseModel classes do not inherit from it via default.

Motivation (including in docs file added):

image

Copy link
Contributor

github-actions bot commented Oct 2, 2025

Test Results

460 tests  +34   456 ✅ +34   23s ⏱️ -1s
  1 suites ± 0     4 💤 ± 0 
  1 files   ± 0     0 ❌ ± 0 

Results for commit 09988e5. ± Comparison against base commit fa733a0.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 96.12546% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.47%. Comparing base (fa733a0) to head (09988e5).

Files with missing lines Patch % Lines
ccflow/utils/path_resolve_spec.py 83.48% 10 Missing and 8 partials ⚠️
ccflow/utils/mixin.py 88.46% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   95.42%   95.47%   +0.05%     
==========================================
  Files         120      126       +6     
  Lines        6754     7295     +541     
  Branches      427      464      +37     
==========================================
+ Hits         6445     6965     +520     
- Misses        203      215      +12     
- Partials      106      115       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

result = handler(values)

# Attach debug metadata if spec was provided
if isinstance(result, object) and spec is not None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe there should be an option to disable this?

values[k] = v
return values
else:
conflicts = [k for k, v in mapping.items() if k in values and values[k] != v]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

arguably we could do this check in the in every case, and log the results.

@ptomecek
Copy link
Collaborator

This is a relatively big change, and I have to think about other ways of accomplishing this.

For example, instead of the configs referencing python paths of other configs, if those other configs were in the registry already, then the existing mechanisms would work to reference them. Then the question becomes how to take some model instances that are defined in python and add them as named objects to the registry. I think this may be doable leveraging hydra's _call_ abilities (i.e. instead of _target_) - i.e. you call a function in python that returns you some object and stick that in the registry under a given name. Then other registry objects can reference it as usual, and the logic for instantiating those objects just lives in python functions...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants