-
Notifications
You must be signed in to change notification settings - Fork 5
Add placeholder tests for streams and operations #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds placeholder tests for streams and operations while also performing minor package refactors and API enhancements.
- Introduces auto-generated tests (placeholders) for streams and operations.
- Refactors package structure and renames modules (e.g. mapper → mappers).
- Adds a new TransferCacher and extends hashing functionality with a name_override parameter.
Reviewed Changes
Copilot reviewed 90 out of 90 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/orcabridge/hashing/string_cachers.py | Added TransferCacher and introduced Redis logging for caching. |
| src/orcabridge/hashing/hashing_legacy.py | Updated inline comments for parameter renaming. |
| src/orcabridge/hashing/files.py | Reordered and adjusted imports. |
| src/orcabridge/hashing/file_hashers.py | Reordered import statements. |
| src/orcabridge/hashing/defaults.py | Added a helper to retrieve a composite hasher with a cacher. |
| src/orcabridge/hashing/core.py | Introduced name_override parameter and refined function hashing logic. |
| src/orcabridge/dj/*.py | Updated import paths and adjusted namespace references. |
| src/orcabridge/base.py | Made Operation and Tracker abstract; added claims_unique_tags logic. |
| pyproject.toml | Added new dependencies for pandas, pyyaml, pyarrow, polars, etc. |
| notebooks/*.ipynb | Minor adjustments to import statements and formatting. |
Comments suppressed due to low confidence (3)
src/orcabridge/hashing/core.py:777
- The function signature for 'get_function_signature' was updated with the 'name_override' parameter, but the docstring does not reflect this change. Please update the docstring to describe the new parameter and its purpose.
def get_function_signature(
src/orcabridge/base.py:310
- The docstring for 'claims_unique_tags' in SyncStream indicates that the method may return None if uniqueness cannot be determined, yet the signature specifies a boolean return type. Please clarify the intended behavior and update the docstring or return type accordingly.
def claims_unique_tags(self, *, trigger_run=True) -> bool:
src/orcabridge/hashing/string_cachers.py:685
- The logger is used in this file without an import or initialization, which could lead to a runtime error. Consider adding 'import logging' and initializing a logger (e.g., 'logger = logging.getLogger(name)') at the top of the file.
logger.info(f"Retrieved cached value from Redis for key {cache_key}")
brian-arnold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a brief look through.
mapper->mappersto reflect the fact it is a collection of multiple mappers).claims_unique_tagsto serve as a mechanism to determine claimed uniqueness of the tag values in a stream. Makes use of the Stream -> Invocation -> Operation -> Stream -> ... recursive patterns used in keys and identity_structure to infer the uniqueness of tags by climbing up the processing pipeline chain.FunctionPodandpipelinepickleable.