-
Notifications
You must be signed in to change notification settings - Fork 8
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
Improve search for fields when they are not directly in the appdef #386
Comments
I generally don't understand how such an algorithm should find out the intended match. See e.g. DATA and AXISNAME in NXdata. As soon as there are more than one unnamed Field/Group, the match cannot be unique, no? As this can lead to wrong behavior then, I would suggest to not do any automatic concept fitting, but always require naming the concept for unnamed fields. |
I agree in most cases this is problematic and this should also be reflected in the changes to the parts above.
This ones are even more special as we do not even have an NXclass attribute available. However, there is a special treatment for
For this we should remove that the algorithm tries to namefit if it doesn't find a fitting exact name and only allow association based on uppercase parts in the path. It eventually turned out that is a design flaw that the algorithm even tries (as there is no benefit, because we almost always have multiple upper case concepts we could fit to). |
When we try to found out if a key is documented in any of the parent appdefs or base classes we check all children names and then try to namefit the best match. However, this part does not into account probable NX classes provided with the dict.
get_all_direct_children_names
has annx_class
parameter now which can be used.A second problem is that concept may be shielded. We had this for
NXmpes/NXsample/temperature(NXenvironment)
andNXsample/temperature(NX_FLOAT)
, which in principle should both be visible butget_all_direct_children_names
is just a set with names so it only shows the one closest to the appdef.This is the code part which needs to be updated:
pynxtools/src/pynxtools/dataconverter/validation.py
Lines 512 to 513 in e0d3184
Probably it's best to search through the inheritance chain with lxml directly, instead of using
get_all_direct_children_names
in combination withsearch_add_child_for
, which just adds the node based on name. With lxml the node should be searched, namefitted and theNexusNode
shall be created withadd_node_from
, which takes an xml element directly (hence no ambiguity). It is a similar technique to what I employed for the sibling namefitting:pynxtools/src/pynxtools/dataconverter/nexus_tree.py
Line 634 in e0d3184
cc @RubelMozumder @mkuehbach (as new maintainers of the validation part)
The text was updated successfully, but these errors were encountered: