python-odml
python-odml copied to clipboard
G-Node
Section attributes
Currently odml.Section() takes name, type, parent and definition.
For me parent does not make sense here. Especially since odml.Property() does not have a parent args.
Another thing: a Section object has a reference attribute, but it can't be defined when creating the Section like it can be done for a Property. In general I think that the reference (if interpreted as reference from where metadata are coming from) is an attribute which should only belong to Properties and not Sections. A Section could contain Properties with metadata coming from different sources.
What do you think?
I think the parent for the section is when a section has various subsections under it. So that section becomes the parent. For the next one, I also think if the reference means from where the metadata comes, it should essentially be with property/value as the property-value pair contains the actual base metadata, and the purpose of the reference in section, thus can be satisfied with property having the reference attribute.
@lzehl Your views?
@mohit-0212:
parent for Section: I think one should definitely be able to get the parent of an Section object. So far the parent was and is still set automatically when a Section is appended to a Document or another Section. As it is for a Property when it is attached to a Section.
I'm not sure if it is necessary and not even problematic when a parent can be provided actively by the user as arg when creating a Section. Currently this also does not work correctly. For example:
s = odml.Section ('S', parent=odml.Document())
print(s)
<Section A[None] (0)>
print(s.parent)
<Doc None by None (0 sections)>
Although the section has the Doc as parent the Doc does not know about his child (holds also for a predefined parent).
Furthermore, if the section s is appended to another object the parent is overwritten.
If we leave this parent arg for Sections it should update the parent of its child, and the same arg should be possible when creating a Property.
Thoughts?
reference: I agree. This is exactly my point/question. Not sure though for what reference was intended for... I would like to have a possibility to refer to the metadata source. I think this is very helpful for managing experimental metadata.
@msuzen:
To your first point:
I'm not sure if a unique identifier is necessary to solve this issue, although of course it could be very helpful. As I said I would prefere using reference really as possibility to state the source of metadata. I create an odml for metadata originating from 13 different sources. Knowing where to look if something is weird is very convenient! To associate objects via a unique identifier I would prefer a new attribute.
To your second point: This is not true! When creating an odml, it SHOULD be possible to create all objects first as standalones. If you build then the tree via append, the parent IS SET automatically for all objects! This should NOT be changed!
So the reference attribute is indeed intended to state the origin of a section, or a value. This can be a more formal definition, might also be a reference to a database entry and what else. We can discuss its usefulness, but this is what is was meant for. We can easily have it as a keyword argument in the constructor. If this is what you are missing.
To have the parent as an optional argument in the constructor of the section class is a shortcut that saves one line of code. If the child does not appear in the parent's list of children may be considered a bug. I agree that we should aim for the similar functionality in Properties and Sections. But again, passing the parent is always optional and may be None for stand-alone object creation.
Regarding the unique ID, I am sure we have it in the roadmap. There were, however, some doubts about the usefulness. In nix, we basically never work with the id and human users definitely prefer names. It would be not big deal to introduce them for unique object identification.
The having a stand-alone attribute, imho, is not required, s.parent is None or something similar should give the desired information.
@jgrewe: for parent issue: I agree! To have parent for Sections as optional kwarg would be nice. But as you say it needs to work correctly (update of children ofthe stated parent) and should be also possible for Properties.
@msuzen I'm not sure if I follow correctly. Did you mean with standalone that a Section has a parent but is not correctly appended to it?
Saving chunks e.g. just a Section with its children is not possible so far correct? I'm also not sure if this is necessary. So far I created a document for such a case and used it to define the chunk I saved.
@msuzen I understand now :) thanks for the clarification in #95.
The parent and reference issue still stands, though. I could have a look, but I'm not sure if I can solve this according to the discussion/suggestion above. To summarize (please correct me if I misunderstood the outcome of our discussion):
- reference attribute should be added as optional *args of a Section
- parent attribute should be added as optional *args of a Property
- if parent is provided as *args, the stated parent needs to updated about the new child
I'll let you know tomorrow if I can take this issue.
@rickskyy Sure! Would you like to take over this issue?
One should wait if the others agree with my summary, though :)
@lzehl I think your summary is the logical solution to the problem you explained before.
@lzehl Adding some comments to the second point:
parent attribute should be added as optional *args of a Property
Property has attribute _section which performs the role of a parent. I suppose we can add optional parent attribute to *args of Property, validate it and store its value in _section.
@msuzen the roadmap is sketched in issue #45
@rickskyy when working on this, it would be good to use the same naming scheme, i.e. call the keyword argument parent in both cases. Further, please add test cases for this functionality. Thanks!
An example how to use this for a Section is in the updated tutorial (assuming already the bug is fixed that the parent is updated about the new child)
PR #98 was only merged into master. The issues described above should also be solved for v1.3.