Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Incorrect operational guidance could cause unintended data replacement and failures when following the examples or upgrading.
4 open findings
What changed in this PR
Reorganizes the Curator administrator guide around creating record-based and file-based metadata workflows in Synapse.
Changes:
- Adds annotated setup examples and sample output.
- Expands parameter, access-control, and return-value references.
- Replaces detailed validation walkthroughs with a concise property reference.
| File | Description |
|---|---|
| docs/guides/extensions/curator/metadata_curation.md | Restructures workflow setup and configuration guidance for administrators. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@cconrad8 This removes a lot of content. Are you sure that's a good idea? |
| # False (default): Disable derived annotations. | ||
| enable_derived_annotations=False, | ||
|
|
||
| ### Getting detailed validation results |
There was a problem hiding this comment.
Removing this breaks a link in docs/guides/extensions/curator/compute_tasks.md
There was a problem hiding this comment.
Some comments about formatting changes and potential Python client doc links / Synapse API doc links that could be added. It will be helpful to visit the site here to see how your documentation gets rendered for your PR.
I also think some of these contents are similar to what we already have for create_record_based_metadata_task() and create_file_based_metadata_task(). Should we just have links to those functions instead of listing out their shared parameters / common parameters?
|
|
||
| Use this when metadata describes individual data files and is stored as annotations directly on each file. | ||
| > [!IMPORTANT] | ||
| > **Before running:** Replace `folder_id` and `schema_uri`. Make sure `upsert_keys` and `column_order` use property names defined in your registered JSON Schema. |
There was a problem hiding this comment.
I think it might be better to have folder_id, and schema_uri up front like this:
SCHEMA_URI = "MyOrganization-BiospecimenMetadata-1.0.0" --> replace with your schema uri
FOLDER_ID = ""
And then in the code:
schema_uri=SCHEMA_URI,
so that users could replace them more easily.
There was a problem hiding this comment.
Also warn users about potentially updating column_order too?
There was a problem hiding this comment.
@linglp can you explain more your suggestions?
for column order i thought the warning was already there and not sure i know what you mean by moving it up front - I thought the parameters were all labeled clearly but please suggest if otherwise!
| - [How to Enter and Update Metadata for a Curation Task](metadata_contribution.md) - The contributor-facing companion to this guide | ||
| - [JSON Schema Tutorial](../../../tutorials/python/json_schema.md) - Learn how to register schemas | ||
| - [Schema Registry](https://synapse.org/Synapse:syn69735275/tables/) - Browse available schemas | ||
| The detailed report can be retrieved with **`RecordSet.get_detailed_validation_results()`**. If no report is available, the method returns `None`. Confirm that the schema is bound and the grid has been exported back to the `RecordSet`. |
There was a problem hiding this comment.
I think here you could also link it to the synapse python client doc: https://python-docs.synapse.org/en/stable/reference/experimental/async/curator/?h=get_detailed_validation_results#synapseclient.models.RecordSet.get_detailed_validation_results_async
thomasyu888
left a comment
There was a problem hiding this comment.
LGTM! I think there are still some things leftover here, that I'll take a look at soon
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Tom Yu <thomas.yu@sagebase.org>
Co-authored-by: Tom Yu <thomas.yu@sagebase.org>
Co-authored-by: Tom Yu <thomas.yu@sagebase.org>
Co-authored-by: Tom Yu <thomas.yu@sagebase.org>
The metadata curation guide no longer has a validation results section, so point to Step 8 of the contributor guide instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thomasyu888
left a comment
There was a problem hiding this comment.
A few remaining items: leftover drafting wording, one inaccurate upgrade note, mismatched example IDs, and the broken inbound link from compute_tasks.md.
| - **Synapse Python Client:** [Install the client](https://python-docs.synapse.org/en/stable/tutorials/installation/) with the Curator extras (`pip install --upgrade "synapseclient[curator]"`) and [configure your credentials](https://python-docs.synapse.org/en/stable/tutorials/authentication/). | ||
| - **Project and folder permissions:** Have **Edit permissions or higher** for the Synapse project and folder where you will manage metadata. Note the folder's Synapse ID for use in the examples. | ||
| - **JSON Schema:** Browse [existing schemas](https://sage-bionetworks.github.io/core-models/) or create your own. You will need a **versioned schema URI**, such as `MyOrganization-MySchema-1.0.0`. For details, see the [Schema Operations Guide](https://python-docs.synapse.org/en/stable/guides/extensions/curator/schema_operations/). |
There was a problem hiding this comment.
Nit: relative links resolve within the PR preview / versioned docs, and match how the other curator guides link (e.g. compute_tasks.md).
| - **Synapse Python Client:** [Install the client](https://python-docs.synapse.org/en/stable/tutorials/installation/) with the Curator extras (`pip install --upgrade "synapseclient[curator]"`) and [configure your credentials](https://python-docs.synapse.org/en/stable/tutorials/authentication/). | |
| - **Project and folder permissions:** Have **Edit permissions or higher** for the Synapse project and folder where you will manage metadata. Note the folder's Synapse ID for use in the examples. | |
| - **JSON Schema:** Browse [existing schemas](https://sage-bionetworks.github.io/core-models/) or create your own. You will need a **versioned schema URI**, such as `MyOrganization-MySchema-1.0.0`. For details, see the [Schema Operations Guide](https://python-docs.synapse.org/en/stable/guides/extensions/curator/schema_operations/). | |
| - **Synapse Python Client:** [Install the client](../../../tutorials/installation.md) with the Curator extras (`pip install --upgrade "synapseclient[curator]"`) and [configure your credentials](../../../tutorials/authentication.md). | |
| - **Project and folder permissions:** Have **Edit permissions or higher** for the Synapse project and folder where you will manage metadata. Note the folder's Synapse ID for use in the examples. | |
| - **JSON Schema:** Browse [existing schemas](https://sage-bionetworks.github.io/core-models/) or create your own. You will need a **versioned schema URI**, such as `MyOrganization-MySchema-1.0.0`. For details, see the [Schema Operations Guide](schema_operations.md). |
| print(f"RecordSet ID: {record_set.id}") | ||
| print(f"CurationTask ID: {curation_task.task_id}") | ||
| ``` | ||
| **Example output** |
There was a problem hiding this comment.
Nit: blank line between the closing code fence and the next paragraph (matches the file-based example below).
| **Example output** | |
| **Example output** |
|
|
||
| ## Complete example script | ||
|
|
||
| Here's the full script that demonstrates both workflow types: | ||
| !!! info | ||
|
|
There was a problem hiding this comment.
Nit: extra blank line and trailing whitespace after !!! info.
| ## Complete example script | |
| Here's the full script that demonstrates both workflow types: | |
| !!! info | |
| !!! info | |
| # After Grid export (either via web UI or programmatically) | ||
| # retrieve the updated RecordSet | ||
| record_set = RecordSet(id="syn987654321").get() | ||
| These parameters are accepted by both `create_record_based_metadata_task()` and `create_file_based_metadata_task()`. |
There was a problem hiding this comment.
Following up on @linglp's comment: linking to the API reference makes it the authoritative source, so these tables don't silently drift from the docstrings.
| These parameters are accepted by both `create_record_based_metadata_task()` and `create_file_based_metadata_task()`. | |
| These parameters are accepted by both [create_record_based_metadata_task][synapseclient.extensions.curator.create_record_based_metadata_task] and [create_file_based_metadata_task][synapseclient.extensions.curator.create_file_based_metadata_task]. See the API reference for each function for the complete, up-to-date signature. |
| !!! note | ||
|
|
||
| Row 1: | ||
| Error: 2 schema violations found | ||
| ValidationError: ["#/genotype: expected type: String, found: Null","#/dateBirth: expected type: String, found: Long"] | ||
| The supplied API reference indicates that returning objects is planned to become the default in **v5.0.0**, when `return_entities` is also scheduled for removal. |
There was a problem hiding this comment.
"The supplied API reference" is leftover drafting wording (the same thing @andrewelamb flagged). It also repeats the info box above the file-based example.
| !!! note | |
| Row 1: | |
| Error: 2 schema violations found | |
| ValidationError: ["#/genotype: expected type: String, found: Null","#/dateBirth: expected type: String, found: Long"] | |
| The supplied API reference indicates that returning objects is planned to become the default in **v5.0.0**, when `return_entities` is also scheduled for removal. | |
| !!! note | |
| Returning objects is planned to become the default in **v5.0.0**, when the `return_entities` parameter will be removed. |
| | Record-based with `create_grid=False` (*recommended*) | `RecordSet`, `CurationTask` | | ||
| | Record-based with `create_grid=True` (*current default in supplied signature; deprecated*) | `RecordSet`, `CurationTask`, `Grid` | | ||
| | File-based with `return_entities=True` (*recommended*) | `EntityView`, `CurationTask` | | ||
| | File-based with `return_entities=False` (*current default in supplied signature; deprecated*) | `EntityView` ID, task ID | |
There was a problem hiding this comment.
Same "supplied signature" leftover wording.
| | Record-based with `create_grid=False` (*recommended*) | `RecordSet`, `CurationTask` | | |
| | Record-based with `create_grid=True` (*current default in supplied signature; deprecated*) | `RecordSet`, `CurationTask`, `Grid` | | |
| | File-based with `return_entities=True` (*recommended*) | `EntityView`, `CurationTask` | | |
| | File-based with `return_entities=False` (*current default in supplied signature; deprecated*) | `EntityView` ID, task ID | | |
| | Record-based with `create_grid=False` (*recommended*) | `RecordSet`, `CurationTask` | | |
| | Record-based with `create_grid=True` (*current default; deprecated*) | `RecordSet`, `CurationTask`, `Grid` | | |
| | File-based with `return_entities=True` (*recommended*) | `EntityView`, `CurationTask` | | |
| | File-based with `return_entities=False` (*current default; deprecated*) | `EntityView` ID, task ID | |
| !!! note | ||
|
|
||
| ```python | ||
| from synapseclient import Synapse | ||
| from synapseclient.models import CurationTask | ||
| **Keep `return_entities=True` and `create_grid=False`.** The current defaults (`return_entities=False` and `create_grid=True`) are deprecated and scheduled for removal in **Synapse Python Client v5.0.0**. Setting these values explicitly keeps your scripts working the same way after that release. |
There was a problem hiding this comment.
Correction to my earlier suggestion (and the point Copilot raised): explicitly passing these arguments does not keep scripts working after v5.0.0. Both parameters are being removed, so a script that still passes them will fail with a TypeError.
| !!! note | |
| ```python | |
| from synapseclient import Synapse | |
| from synapseclient.models import CurationTask | |
| **Keep `return_entities=True` and `create_grid=False`.** The current defaults (`return_entities=False` and `create_grid=True`) are deprecated and scheduled for removal in **Synapse Python Client v5.0.0**. Setting these values explicitly keeps your scripts working the same way after that release. | |
| !!! note | |
| **Set `create_grid=False` and `return_entities=True` for now.** The current defaults (`create_grid=True` and `return_entities=False`) are deprecated. In **Synapse Python Client v5.0.0**, both parameters will be removed and the helpers will always return the *recommended* values shown above. When you upgrade to v5.0.0, remove these arguments from your scripts, because passing a removed parameter raises a `TypeError`. |
| | `curation_task.task_properties.suggested_authorization_mode` | Access-mode recommendation set by `authorization_mode`. | | ||
|
|
||
| ### API Documentation | ||
| ### Record-Based Validation Properties |
There was a problem hiding this comment.
Broken inbound link (re: @andrewelamb's comment): compute_tasks.md linked to metadata_curation.md#getting-detailed-validation-results, which this PR removes. Since that file is outside this diff, I pushed the fix directly in 1136c06: it now points to Step 8: Review your validation results in the contributor guide.
I used Step 8 rather than Step 6 ("Validate your edits in-session") because that sentence is about the saved report read from validation_file_handle_id after export, which Step 8 covers. Step 6 is in-session validate_rows() before a push.
Separately, metadata_contribution.md has two ### Step 6 headings ("Import edited record-based metadata" and "Validate your edits in-session"), so the steps after them are off by one. Not part of this PR, but worth fixing in a follow-up.
| ### Record-Based Validation Properties | ||
|
|
||
| - [query_schema_registry][synapseclient.extensions.curator.query_schema_registry] - Search for schemas in the registry | ||
| - [create_record_based_metadata_task][synapseclient.extensions.curator.create_record_based_metadata_task] - Create RecordSet-based curation workflows | ||
| - [create_file_based_metadata_task][synapseclient.extensions.curator.create_file_based_metadata_task] - Create EntityView-based curation workflows | ||
| - [RecordSet.get_detailed_validation_results][synapseclient.models.RecordSet.get_detailed_validation_results] - Get detailed validation results for RecordSet data | ||
| - [Grid.create][synapseclient.models.curation.Grid.create] - Create a Grid session from a RecordSet or EntityView | ||
| - [Grid.export_to_record_set][synapseclient.models.curation.Grid.export_to_record_set] - Export Grid data back to RecordSet and generate validation results | ||
| - [Folder.bind_schema][synapseclient.models.Folder.bind_schema] - Bind schemas to folders | ||
| - [Folder.validate_schema][synapseclient.models.Folder.validate_schema] - Validate folder schema compliance | ||
| - [CurationTask.list][synapseclient.models.CurationTask.list] - List curation tasks in a project | ||
| - [CurationTask.set_task_state][synapseclient.models.CurationTask.set_task_state] - Update the lifecycle state of a curation task | ||
| After metadata is exported from the grid back to a schema-bound `RecordSet`, Synapse generates a saved **row-level validation report**. Administrators can use the following fields to review the results. | ||
|
|
||
| ### Related Documentation | ||
| | Property or report column | Description | | ||
| | --- | --- | | ||
| | `record_set.validation_file_handle_id` | Identifies the saved validation report after an export generates validation results. | | ||
| | `row_index` | Identifies the row associated with a validation result. | | ||
| | `is_valid` | Whether the row passed validation: `True` or `False`. | | ||
| | `validation_error_message` | Summary of validation errors for the row. | | ||
| | `all_validation_messages` | All schema-validation messages for the row. | | ||
|
|
||
| - [How to Enter and Update Metadata for a Curation Task](metadata_contribution.md) - The contributor-facing companion to this guide | ||
| - [JSON Schema Tutorial](../../../tutorials/python/json_schema.md) - Learn how to register schemas | ||
| - [Schema Registry](https://synapse.org/Synapse:syn69735275/tables/) - Browse available schemas | ||
| After contributors synchronize (push) the grid back to the `RecordSet`, fetch the latest version before retrieving the detailed report. Import `RecordSet` with `from synapseclient.models import RecordSet`, then use `record_set = RecordSet(id=record_set.id).get(synapse_client=syn)` to load the latest `validation_file_handle_id`. Retrieve the report with `record_set.get_detailed_validation_results(synapse_client=syn)`. If the method returns `None`, confirm that the schema is bound and the grid changes have been pushed to the `RecordSet`. |
There was a problem hiding this comment.
Since the validation walkthrough already lives in the contributor guide, I'd point there instead of keeping a partial copy here.
| ### Record-Based Validation Properties | |
| - [query_schema_registry][synapseclient.extensions.curator.query_schema_registry] - Search for schemas in the registry | |
| - [create_record_based_metadata_task][synapseclient.extensions.curator.create_record_based_metadata_task] - Create RecordSet-based curation workflows | |
| - [create_file_based_metadata_task][synapseclient.extensions.curator.create_file_based_metadata_task] - Create EntityView-based curation workflows | |
| - [RecordSet.get_detailed_validation_results][synapseclient.models.RecordSet.get_detailed_validation_results] - Get detailed validation results for RecordSet data | |
| - [Grid.create][synapseclient.models.curation.Grid.create] - Create a Grid session from a RecordSet or EntityView | |
| - [Grid.export_to_record_set][synapseclient.models.curation.Grid.export_to_record_set] - Export Grid data back to RecordSet and generate validation results | |
| - [Folder.bind_schema][synapseclient.models.Folder.bind_schema] - Bind schemas to folders | |
| - [Folder.validate_schema][synapseclient.models.Folder.validate_schema] - Validate folder schema compliance | |
| - [CurationTask.list][synapseclient.models.CurationTask.list] - List curation tasks in a project | |
| - [CurationTask.set_task_state][synapseclient.models.CurationTask.set_task_state] - Update the lifecycle state of a curation task | |
| After metadata is exported from the grid back to a schema-bound `RecordSet`, Synapse generates a saved **row-level validation report**. Administrators can use the following fields to review the results. | |
| ### Related Documentation | |
| | Property or report column | Description | | |
| | --- | --- | | |
| | `record_set.validation_file_handle_id` | Identifies the saved validation report after an export generates validation results. | | |
| | `row_index` | Identifies the row associated with a validation result. | | |
| | `is_valid` | Whether the row passed validation: `True` or `False`. | | |
| | `validation_error_message` | Summary of validation errors for the row. | | |
| | `all_validation_messages` | All schema-validation messages for the row. | | |
| - [How to Enter and Update Metadata for a Curation Task](metadata_contribution.md) - The contributor-facing companion to this guide | |
| - [JSON Schema Tutorial](../../../tutorials/python/json_schema.md) - Learn how to register schemas | |
| - [Schema Registry](https://synapse.org/Synapse:syn69735275/tables/) - Browse available schemas | |
| After contributors synchronize (push) the grid back to the `RecordSet`, fetch the latest version before retrieving the detailed report. Import `RecordSet` with `from synapseclient.models import RecordSet`, then use `record_set = RecordSet(id=record_set.id).get(synapse_client=syn)` to load the latest `validation_file_handle_id`. Retrieve the report with `record_set.get_detailed_validation_results(synapse_client=syn)`. If the method returns `None`, confirm that the schema is bound and the grid changes have been pushed to the `RecordSet`. | |
| ### Validation Results | |
| After contributors push grid changes back to a schema-bound `RecordSet`, Synapse generates a row-level validation report. To retrieve and interpret the report, see [Step 8: Review your validation results](metadata_contribution.md#step-8-review-your-validation-results) in the contributor guide. |


Problem:
Solution:
Testing: