-
Notifications
You must be signed in to change notification settings - Fork 72
[SYNPY-1577] Submission View #1192
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
Co-authored-by: BryanFauble <[email protected]>
…ion Views, allowing users to create, manage, and query collections of evaluation queue submissions. This provides a tabular view of submissions similar to other view types in Synapse.
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 comprehensive support for a new SubmissionView model to the Synapse Python Client. Key changes include:
- Introducing the SubmissionView model and integrating it in model imports, constants, and factory mappings.
- Updating the client submission logic to support both dataclass and dictionary representations.
- Adding a complete tutorial script and updated documentation for both synchronous and asynchronous interfaces.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
synapseclient/models/services/storable_entity_components.py | Added SubmissionView to type unions and updated activity change logic. |
synapseclient/models/mixins/table_components.py | Included SubmissionView in the list of classes that contain row etags. |
synapseclient/models/init.py | Exported SubmissionView accordingly. |
synapseclient/core/constants/concrete_types.py | Added SUBMISSION_VIEW constant to the concrete types. |
synapseclient/client.py | Updated submit handling for dataclass-based entities with SubmissionView. |
synapseclient/api/entity_factory.py | Mapped the concrete type SUBMISSION_VIEW to the SubmissionView model. |
mkdocs.yml | Updated navigation to include SubmissionView documentation. |
docs/tutorials/python/tutorial_scripts/submissionview.py | Added a comprehensive tutorial script for SubmissionView usage. |
docs/tutorials/python/submissionview.md | Created tutorial documentation for SubmissionView. |
docs/reference/experimental/sync/submissionview.md | Added new API reference documentation for SubmissionView (sync). |
docs/reference/experimental/async/submissionview.md | Added new API reference documentation for SubmissionView (async). |
.readthedocs.yaml | Modified reference ranking settings for improved documentation ordering. |
|
||
# Step 3: Create and submit a file to the evaluation queue | ||
with tempfile.NamedTemporaryFile( | ||
mode="w", suffix=".txt", delete=True, delete_on_close=False |
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.
The use of both 'delete' and 'delete_on_close' parameters in NamedTemporaryFile is non-standard and may lead to unexpected behavior; consider using only one appropriate parameter based on your intended file deletion behavior.
mode="w", suffix=".txt", delete=True, delete_on_close=False | |
mode="w", suffix=".txt", delete=True |
Copilot uses AI. Check for mistakes.
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.
Bad suggestion. Using both means the file is deleted when leaving the content managed "with" statement, but not deleted the moment we stop writing to the file through I/O
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.
🔥 LGTM! Took a peak at the tests and also tutorial. Thanks for that! Will defer to the team for the final review. It does seem like tests are taking awhile to run now due to all these calls. 😅
I think we could have others do the conversion of the challenge services into OOP when we get there.
…tiple entities and move to ColumnMixin. Making SubmissionView access controllable
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.
🔥
Problem:
Solution:
Testing:
tests/integration/test_submissionview.py
tests/integration/async/test_submissionview_async.py