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

Make schemes safe against user changes #63

Open
hagenw opened this issue Apr 27, 2021 · 2 comments · May be fixed by #279
Open

Make schemes safe against user changes #63

hagenw opened this issue Apr 27, 2021 · 2 comments · May be fixed by #279
Labels
wontfix This will not be worked on

Comments

@hagenw
Copy link
Member

hagenw commented Apr 27, 2021

At the moment a user can change the dtype or labels of a scheme, but this will not automatically update the related tables.
So we should make those attributes properties that return a copy and provide setter functions. BTW, you can add a setter function to a Python property, so a user should be able to do something like schemes['my-scheme'].labels[0] = 'a' if we want to allow for it.

@frankenjoe
Copy link
Collaborator

frankenjoe commented Jan 17, 2023

At some point we have introduced Scheme.replace_labels() to offer the user a safe way to replace labels. This does of course not solve the issue that a user might do schemes['my-scheme'].labels[0] = 'a' which is not safe. However, it is not straightforward to rename the attribute labels to e.g. _labels and instead introduce a property labels as in that case it would not be stored to YAML any longer. So I wonder if we should simply stick with Scheme.replace_labels() as the proper way to change labels, just like a user should e.g. not directly modified Table.df.

@hagenw hagenw added the wontfix This will not be worked on label Jan 18, 2023
@hagenw
Copy link
Member Author

hagenw commented Jan 18, 2023

I agree now that we have Scheme.replace_labels() users should no longer be tempted to replace labels directly.
I would also propose to not do anything here.
I mark this issue as wontfix for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants