Skip to content
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

Raise exception if trying to set a dict with no id as property value #193

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

simleo
Copy link
Collaborator

@simleo simleo commented Sep 5, 2024

Closes #190.

Adds a check for dict values without @id to Entity.__setitem__.

Copy link
Contributor

@elichad elichad left a comment

Choose a reason for hiding this comment

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

Looks good, suggest adding some comments

Comment on lines 366 to 370
with open(crate_dir / helpers.METADATA_FILE_NAME, "wt") as f:
json.dump(metadata, f, indent=4)
with pytest.raises(ValueError):
crate = ROCrate(crate_dir)
del metadata["@graph"][2]["badProp"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a couple of comments to indicate the different things that are being tested here? With line 368-9 in particular, it's not intuitive that that part of the test relates to this specific issue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See 1391dc3

@elichad elichad merged commit ad8e816 into ResearchObject:master Sep 11, 2024
12 checks passed
@simleo simleo deleted the handle_set_prop_value_no_id branch September 12, 2024 07:57
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.

Setting/Accessing nested properties on entities without @id
2 participants