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

Error in documentation and asdict #811

Closed
sygout opened this issue Mar 21, 2024 · 2 comments · Fixed by #833
Closed

Error in documentation and asdict #811

sygout opened this issue Mar 21, 2024 · 2 comments · Fixed by #833

Comments

@sygout
Copy link
Collaborator

sygout commented Mar 21, 2024

In the documentation it is specified: uri: An optional unique name identifying the instance

But a dictionary without that key cannot be load from a json file.

The function asdict produces a dictionary that does not contain uri but uuid.

@jesper-friis
Copy link
Collaborator

Agree, it might be little confusing. SOFT5 had slightly different ways to serialise entities and instances as JSON, which DLite inherited. In DLite we call these two serialisation forms for single-entity form (entities) and multi-entity form (instances).

The asdict() method has a boolean option called single for selecting which serialisation form to use. The default value is True, which was an unfortunate choice, since not all plugins supports parsing instances in single-entity form.

PR #833 changes the default to single-entity form for entities and multi-entity form for instances. This may be easier to use, but changes the behaviour of DLite and may break existing code.

An alternative would be to always use the more general multi-entity form as the default.

@sygout
Copy link
Collaborator Author

sygout commented May 1, 2024

I think that it is necessary to clarify what DLite is supposed to do before aligning one way or the other. As it might be a breaking change, it would require a new version and so it might be worth considering what that version should look like.

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 a pull request may close this issue.

2 participants