Skip to content

Conversation

@EmanAbdelhaleem
Copy link
Contributor

Metadata

Details

enhance the docstrings of flows core public functions, add examples, parameter default, parameter type..etc

@codecov-commenter
Copy link

codecov-commenter commented Dec 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.72%. Comparing base (c5f68bf) to head (de22ee7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1569      +/-   ##
==========================================
- Coverage   53.02%   52.72%   -0.31%     
==========================================
  Files          36       36              
  Lines        4326     4326              
==========================================
- Hits         2294     2281      -13     
- Misses       2032     2045      +13     

☔ 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.

@EmanAbdelhaleem EmanAbdelhaleem marked this pull request as ready for review January 8, 2026 21:29
Copy link
Collaborator

@SimonBlanke SimonBlanke left a comment

Choose a reason for hiding this comment

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

Good job. Just a few questions in the code.

Side Effects
------------
- None: results are fetched and returned; no local state is modified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this true? Are there local connection states? And what about caching? 'no local state is modified' is a strong claim.

Copy link
Contributor Author

@EmanAbdelhaleem EmanAbdelhaleem Jan 20, 2026

Choose a reason for hiding this comment

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

I meant that it's a read-only operation that doesn't change variables, database or files. I am not sure about flows cuz I haven't gone very deep with its implementation, However, in evaluations, the list function has no caching as far as I know, and I think mostly this is the case for flows too.

Would you suggest modifying this to
None: read-only operation ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

'Read only' would be clear. It is easier to understand

``strict_version`` is True, an exception will be raised.
strict_version : bool, optional (default=True)
When ``reinstantiate`` is True, whether to enforce exact version
requirements for the extension/model. If False, a fallback flow may
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not a fallback flow though, is it? It is a new flow:

return new_flow

Fallback would suggest some sort of 'backup' flow. Is this the case here (it might be)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope it's not the case, I meant by fallback that it's an alternative flow, but I think fallback might be misleading, I think I will just make it new flow

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay new flow then. It is better to be more precise.

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.

4 participants