Skip to content

[SYNPY-1838] prepare function for removal of Grid creation#1380

Merged
andrewelamb merged 11 commits into
developfrom
prepare_create_record_based_metadata_task_function_for_grid_removal
May 25, 2026
Merged

[SYNPY-1838] prepare function for removal of Grid creation#1380
andrewelamb merged 11 commits into
developfrom
prepare_create_record_based_metadata_task_function_for_grid_removal

Conversation

@andrewelamb
Copy link
Copy Markdown
Contributor

Problem:

Creating the Grid class shouldn't be part of the create_record_based_metadata_task function.
The Grid is returned as part of a tuple, and removing this would be a breaking change.

Solution:

  • A warning about the upcoming breaking change was added.
  • TODOs were added for v5.0.0

@andrewelamb andrewelamb requested a review from a team as a code owner May 11, 2026 20:17
# TODO: https://sagebionetworks.jira.com/browse/SYNPY-1838
# remove this warning
synapse_client.logger.warning(
"The Grid object will no longer be created starting in v5.0.0. "
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great v1 and let me suggest an edit from a user perspective.

From a pure user perspective- it's not clear to me what the grid enables so the impact isn't clear.

@thomasyu888 thomasyu888 requested review from BryanFauble and linglp May 11, 2026 20:48
Copy link
Copy Markdown
Contributor

@linglp linglp left a comment

Choose a reason for hiding this comment

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

LGTM! See my comments below. I am a bit worried that this fix will be too late for users if this is part of v5.0.0 release. I am wondering if you have talked to Christina and if we should move it sooner.

Comment thread synapseclient/extensions/curator/record_based_metadata_task.py Outdated
Comment thread synapseclient/extensions/curator/record_based_metadata_task.py Outdated
@andrewelamb andrewelamb marked this pull request as draft May 12, 2026 19:12
@andrewelamb
Copy link
Copy Markdown
Contributor Author

Converting to draft, we may not actually do this.

@andrewelamb andrewelamb marked this pull request as ready for review May 13, 2026 20:43
@thomasyu888 thomasyu888 marked this pull request as draft May 20, 2026 23:13
@andrewelamb andrewelamb marked this pull request as ready for review May 22, 2026 20:35
Comment thread synapseclient/extensions/curator/record_based_metadata_task.py
@thomasyu888 thomasyu888 changed the title prepare function for removal of Grid creation [SYNPY-1838] prepare function for removal of Grid creation May 22, 2026
Copy link
Copy Markdown
Member

@BryanFauble BryanFauble left a comment

Choose a reason for hiding this comment

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

Small change to update the function args

@andrewelamb andrewelamb requested a review from BryanFauble May 22, 2026 22:08
@andrewelamb andrewelamb merged commit a2442cd into develop May 25, 2026
38 of 51 checks passed
@andrewelamb andrewelamb deleted the prepare_create_record_based_metadata_task_function_for_grid_removal branch May 25, 2026 23:15
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