Skip to content

Metadata can appear to be set directly, but doesn't work: unfriendly to the user #993

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.

Sign up for GitHub

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

Jump to bottom
Open
hyanwong opened this issue Nov 13, 2020 · 7 comments · May be fixed by #3140
Open

Metadata can appear to be set directly, but doesn't work: unfriendly to the user #993

hyanwong opened this issue Nov 13, 2020 · 7 comments · May be fixed by #3140
Labels
enhancement New feature or request Python API Issue is about the Python API
Milestone
Python 1.0

Comments

@hyanwong
Copy link
Member

I see that it looks like I can set metadata properties after creating them, but it doesn't actually update the metadata. I think it should either work, or we should raise an error if possible:

tables = tskit.TableCollection(1)
tables.metadata_schema = tskit.MetadataSchema({"codec":"json"})
tables.metadata = {'foo': {}}
tables.metadata['foo']['bar']=20
ts = tables.tree_sequence()
print(ts.metadata['foo']) # doesn't return 20
@jeromekelleher
Copy link
Member

Thoughts @benjeffery?

@benjeffery
Copy link
Member

Tough one this as supporting the assignment (recursively) is not that simple, possible though I assume. We could return a frozendict, but would need a nice error message. I'll look into supporting the assignment, if it gets too hairy (for example should `del tc.metadata['foo']['bar'] work?) I'll try freezing.

@benjeffery benjeffery added enhancement New feature or request Python API Issue is about the Python API labels Nov 13, 2020
@benjeffery benjeffery added this to the Python upcoming milestone Nov 13, 2020
@jeromekelleher
Copy link
Member

I'd say a frozendict would be a good first step - actual assigmnent will be hairy and would need some more low-level infrastructure for setting values in rows.

@benjeffery
Copy link
Member

Ah, yes. I'd only considered the ts-level case.

@hyanwong
Copy link
Member Author

Making it read-only would be fine, IMO.

@jeromekelleher jeromekelleher mentioned this issue Nov 13, 2021
Open
@jeromekelleher jeromekelleher mentioned this issue Dec 2, 2021
Closed
@petrelharp
Copy link
Contributor

FWIW here's a situation where (I'm not sure but I suspect) things were hard to figure out for a user because it seems like you can just change metadata but it doesn't do anything: https://groups.google.com/g/slim-discuss/c/N2v0fyiYOoQ/m/pHZ9h5v0CAAJ

A natural thing for this user to try would have been

ts.metadata['SLiM']['separate_sexes'] = True
# or perhaps
tables.metadata['SLiM']['separate_sexes'] = True

but those just silently do nothing.

@jeromekelleher
Copy link
Member

Retuning a frozendict is an easy thing to do for 1.0 I think @benjeffery ?

@benjeffery benjeffery linked a pull request Apr 14, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request Python API Issue is about the Python API
Projects
None yet
Milestone
Python 1.0
Development

Successfully merging a pull request may close this issue.

Freeze metadata benjeffery/tskit
4 participants
@benjeffery @petrelharp @jeromekelleher @hyanwong