Skip to content

Reject adding objects owned by another document - #480

Merged
tcmitchell merged 2 commits into
SynBioDex:mainfrom
Shubham-Padkonde:fix/document-ownership
Sep 30, 2026
Merged

tcmitchell merged 2 commits into
SynBioDex:mainfrom
Shubham-Padkonde:fix/document-ownership

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown
Contributor

Fixes #416.

Adding an existing top-level object to a second document currently leaves it in the first document while changing its document reference (and its children's references) to the second. Reject that addition with ValueError before modifying either document, as proposed in the issue discussion.

Regression coverage exercises both single-object and list input, checks that child ownership is preserved, and confirms that removal followed by addition to another document still works. Both second-document cases failed before the fix.

Validation on Python 3.13: python -We -m unittest discover -s test ran 223 tests successfully with one existing expected failure, using the pinned SBOLTestSuite submodule. pycodestyle sbol3 test, pylint sbol3 test (10/10), and git diff --check pass.

Prepared with Codex assistance.

@tcmitchell

Copy link
Copy Markdown
Collaborator

Thanks @Shubham-Padkonde ! This looks good. I ran the GitHub actions on your pull request and it passed all the testing, which includes multiple versions of Python.

Can I ask that you please add some comments to the two unit tests to explain what they are testing? Just an overview comment that says what the unit test does or what it tests for. And please include a link to the GitHub Issue URL as well so we can refer back to it if we ever need to.

Once the comments are added please add a comment here. I should be able to merge this pull request then.

Thanks again!

@tcmitchell

Copy link
Copy Markdown
Collaborator

Thanks for adding comments and links to the new unit tests, and thanks very much for taking on this task!

@tcmitchell
tcmitchell self-requested a review September 30, 2026 13:15
@tcmitchell tcmitchell self-assigned this Sep 30, 2026
@tcmitchell tcmitchell added this to the 1.3 milestone Sep 30, 2026
@tcmitchell
tcmitchell merged commit 08c8382 into SynBioDex:main Sep 30, 2026
8 checks passed
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.

Adding an object to more than one parent Document

2 participants