[SYNPY-1589] Implement "Evaluation" OOP model #1244
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
) * Initialize protocol class + Evaluation dataclass * exposing Evaluation API services. dataclass updates. syntax updates. * dataclass methods are added which wrap API service functions * fixed missing imports and syntax failures * Evaluations can be imported like from synapseclient.models import Evaluation * correct ACL inputs and output types. first pass at fixing async integration tests. * fix import issues * fix type errors. api module and its functions can now be imported individually. test improvements. * add etag to prevent issues from OCC when updating an evaluation * some formatting * refactor: update and create will rely on the attributes changing in the class object itself rather than feeding it into the method. new to_synapse_request * remove principal_id param. cannot be used. adding examples to docstrings * patch all async tests * style * remove unnecessary imports * complete refactor of store/update. store is now used for updating too * add example to Evaluation dataclass * new evaluation protocol class. moved out to protocols subfolder * add synchronous integration tests * new unit tests. updated integration tests. content_source is an immutable field. style * show available access types * remove todo * get logger instance * no need for request_type param in store_evaluation_async. removing CASE 2 for handling ID of new evals. remove integration test * docstring example formatting Co-authored-by: BryanFauble <[email protected]> * update examples in docstrings * the logger called in merge_dataclass_entities is now retrieved from active client * remove _async suffix from evaluation_services functions * style * updated docstrings * store -> create_or_update, updated tests * style * refactor update_acl dataclass method for user qol * give users option to remove principals from ACL * convert sync test methods to async so it calls schedule_for_cleanup fixture properly * style * if/elif -> if/else * iteratively grab the attributes instead * request_type is now a CREATE/UPDATE enum * feed query params directly into client httpx response call so it can parse params with urlencode * style * grab the logger outside of the for-loop --------- Co-authored-by: BryanFauble <[email protected]>
jaymedina
force-pushed
the
synpy-1589-evaluation-model-main
branch
from
eb6e692 to
fc79d2e
Compare
* first draft of docs * style * update tutorial to reflect new code in tutorial script. update formatting of model examples. * add more examples to API reference docs * update tutorial source code * enforce principal_id is not none * move PRINCIPAL_ID assert. update tutorial .md * Update docs/tutorials/python/evaluation.md Co-authored-by: BryanFauble <[email protected]> * force uppercase * fix uppercase force * additional prereq * fix indentation. fix example * change eval names --------- Co-authored-by: BryanFauble <[email protected]>
BryanFauble
reviewed
Oct 23, 2025
BryanFauble
reviewed
Oct 23, 2025
BryanFauble
reviewed
Oct 23, 2025
BryanFauble
approved these changes
Oct 23, 2025
|
Thanks @BryanFauble 🙏 |
linglp
approved these changes
Oct 24, 2025
| active_evaluations = await Evaluation.get_all_evaluations_async(synapse_client=self.syn, active_only=True) | ||
|
|
||
| # THEN the active evaluations should be retrieved | ||
| assert active_evaluations is not None |
There was a problem hiding this comment.
I am a bit worried that the test here is not sufficient. If active_evaluations returns an empty list, this will also pass the test. Is there a way to make sure all the statuses of evaluations are "active" in active_evaluations?
| evaluations_list = Evaluation.get_evaluations_by_project(project_id) | ||
|
|
||
| # Let's delete the evaluation we created for this tutorial, and any other evaluations in this project (uncomment below to enable deletion) | ||
| # for evaluation_to_delete in evaluations_list: |
There was a problem hiding this comment.
Intead of having the user going through a loop to delete evaluation, I am thinking if it is possible to just do evaluation.delete() since the evaluation object was created in previous step.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
This work is part of the ongoing effort to refactor the Synapse Python Client using object-oriented principles, following the pattern established in https://sagebionetworks.jira.com/browse/SYNPY-1418.
We currently to not have an OOP model for communications to the Evaluation API services provided by the Synapse platform.
Solution:
The work has been separated into 3 feature branches, with the
*mainbranch being intended for merge intodevelopand the other 2 branches merging into*mainwhen ready.The acceptance criteria has been split up into these 3 branches in the following way.
Branch:
synpy-1589-evaluation-model-mainBranch:
synpy-1589-evaluation-model-functionality(merges into*main)DataClassis introduced following the official attributes of an Evaluation entity on Synapse, viamodels/evaluation.py.api/evaluation_services.py:Create evaluation: https://rest-docs.synapse.org/rest/POST/evaluation.html
Get evaluation by ID: https://rest-docs.synapse.org/rest/GET/evaluation/evalId.html
Get evaluations by entity: https://rest-docs.synapse.org/rest/GET/entity/id/evaluation.html
Get all evaluations: https://rest-docs.synapse.org/rest/GET/evaluation.html
Get available evaluations: https://rest-docs.synapse.org/rest/GET/evaluation/available.html
Find evaluation by name: https://rest-docs.synapse.org/rest/GET/evaluation/name/name.html
Update evaluation: https://rest-docs.synapse.org/rest/PUT/evaluation/evalId.html
Delete evaluation: https://rest-docs.synapse.org/rest/DELETE/evaluation/evalId.html
Get ACL: https://rest-docs.synapse.org/rest/GET/evaluation/evalId/acl.html
Update ACL: https://rest-docs.synapse.org/rest/PUT/evaluation/acl.html
Get permissions: https://rest-docs.synapse.org/rest/GET/evaluation/evalId/permissions.html
DataClassthat wrap around the evaluation servicesBranch:
synpy-1589-evaluation-model-docs(merges into*main)Testing: