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

Parsing HDF5 with attributes ... cannot remove non-attribute item from template #61

Closed
drowenhorst-nrl opened this issue Sep 7, 2022 · 3 comments · Fixed by #74
Closed
Assignees

Comments

@drowenhorst-nrl
Copy link
Contributor

drowenhorst-nrl commented Sep 7, 2022

*** in the context of create template ***
When parsing an HDF5 with attributes, the data item/group that has an attribute cannot be removed from the template. For instance if the item/group has atributes "Bar 1" and "Bar 2", then the source tree appears like :
/Foo/Bar 1*
/Foo/Bar 2*
'/Foo'

You can remove "/Foo/Bar 1" and "/Foo/Bar 2" but not "/Foo"

Note here ... all "remove" actions are using the check boxes in the MetaData tree. See below for perhaps related issue.

@drowenhorst-nrl
Copy link
Contributor Author

Not sure if related but ... with hdf5 files with attributes, attempting to remove rows using the "X" on the create template window, the first click will not remove the selected row, but the top item. Further clicks appear to do nothing.

@cryos cryos self-assigned this Feb 16, 2023
@cryos
Copy link
Collaborator

cryos commented Feb 22, 2023

This seems like a more general problem with attributes not quite fitting the data model. So our simple solution of just adding a * works for the most part, but the new XML parser is suffering from the same issue. I started looking into this last week, and I can see it with XML and HDF5 files.

Looking at the way the tree model works you cannot uncheck "directory" type things that contain other things. The issue as I see it is without attributes an entry is either a containing directory, so it doesn't get its own row in the table, or it is a data entry and it does get an entry. We now have another situation where an entry might be made for a data entry, and that data entry has several attributes it contains.

I think I understand the ez model stuff, but this probably warrants a discussion before we go changing things. Do we want to make attributes special in the data model, or do we want to generalize the nodes that contain things to be checkable/uncheckable in their own right?

@joeykleingers joeykleingers self-assigned this Mar 6, 2023
@joeykleingers
Copy link
Contributor

joeykleingers commented Mar 7, 2023

We had a meeting about this and decided to do the following:

  • Rewrite any parsers that are returning nested file dictionaries to instead return flattened file dictionaries. EzMetadataModel also needs to be updated to remove the logic dealing with unpacking nested file dictionaries. The code in there should simplify quite a bit since it'll just be a matter of parsing each incoming flattened dictionary item into an EzMetaDataEntry object and then appending it to the "entries" list property.

  • Any node (Foo) that has attributes will have a node in the tree called "Foo*" and a parallel node in the tree called "Foo.attributes" in italics. All the attributes for node Foo are listed as children of "Foo.attributes".

cryos added a commit to quantumista/MetaForge that referenced this issue Mar 25, 2023
Issue BlueQuartzSoftware#61 outlined issues with the attributes as implemented. This
switches the attributes to always append '.attrs' to the path to the
attribute entries. This avoids the situation where there is data the
user wants to remove from their template but they want to keep some
attributes. This resolves the issue outlines as the problematic entry is
never emitted from the parser now.
@joeykleingers joeykleingers linked a pull request Mar 27, 2023 that will close this issue
joeykleingers pushed a commit that referenced this issue Mar 27, 2023
* Use a node with .attrs appended for attributes

Issue #61 outlined issues with the attributes as implemented. This
switches the attributes to always append '.attrs' to the path to the
attribute entries. This avoids the situation where there is data the
user wants to remove from their template but they want to keep some
attributes. This resolves the issue outlines as the problematic entry is
never emitted from the parser now.

* Fix a few issues with the XML parser

This addresses the attribute issue in the same way as the HDF5 parser,
using '.attrs' appended to the path before the attribute name. It also
adds some extra logic to only add entries if an XML tag contains
non-whitespace data.

* Update with the new convention for attributes

This updates the documentation to cover the '.attrs' convention
replacing the trailing '*' notation for attributes in formats that
support them. This change was made in the HDF5 and XML parsers.
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.

3 participants