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

Implementation of verattr tag and xml dependence. #137

Merged
merged 6 commits into from
Nov 7, 2021

Conversation

Candoran2
Copy link
Contributor

@Candoran2 Candoran2 commented Oct 22, 2021

Implementation of verattr

I implemented codegen using the verattr tag to designate which variables should be global variables. Previously, anything that had 'version' in the field name was stored on the context, whereas now its explicitly defined.

The verattr tag is now also used to determine how to do global variable assignment. Previously, global variables with the name 'user_version' were always assigned like bitfields. Now, the type (and therefore the tag type) is always stored on the verattr tag, which allows you to determine whether it is a bitfield or not.

I also added an extra check to Union.py write_io to only do assignment of global variables on the read method (since it makes no sense to do global variable assignment on the context when writing the object).

Aside from the aforementioned check, there should be no changes for existing codegen (save one instance where the user_version field was written/stored as a uint, whereas it was actually a VersionInfo bitfield).

Xml dependence

Description

I have implemented external xml dependence via the XInclude scheme. It uses a file path relative to the xml, so that they are not dependent on knowledge of the interpreter. This functionality works recursively.
For a concrete example of such an element:

<xi:include href="../base/base.xml" xmlns:xi="http://www.w3.org/2001/XInclude" xpointer="xpointer(*/*)" />

The href attribute is the file path, relative to the xml's directory. xmlns:xi attribute is necessary for correct reading by ElementTree. The xpointer attribute specifies what elements from the linked xml file should be included. It is constructed such that, if the element were to be evaluated by something like lxml`'s include function, it would be the same as if every child element of the linked xml's root were included in the xml file (almost, since it adds an extra xml:base attribute). The standard ElementTree library unfortunately does not support the xpointer attribute, which means it would include the root niftoolsxml tag. If necessary, it would also be possible to modify codegen such that it could evaluate niftoolsxml tag elements like their children are top-level children of the root, but that seems unnecessary accommodations for functionality that we're circumventing in the first place.

Changes to existing functionality

ovl_base

Common elements between fgm, ovl, ms2, tex and voxelskirt were relocated to the ovl_base format. This is both the xml descriptions and some source files (FixedString.py and ZStringBuffer.py). All their headers now inherit from GenericHeader which was previously located in the ovl xml.

base

Common elements between all formats (except dds) were relocated to the base format. This is just the standard operator tokens and 9 basic types. The reason dds was left alone is that it didn't include the tokens, and didn't have the uint64 basic type.

Usage

Addition of parsed_xmls argument to load_xml.

The optional parsed_xmls argument was added to XmlParser's load_xml function. It's function is to store information about for which xmls codegen has already generated. Omitting it will not prevent codegen from running, but it will make it generate code for included xmls even if they have already been generated. Specify this as a dictionary when generating multiple xmls for performance. This change has already been made to the existing generate_classes function.

Relocation of token elements

The way tokens work requires that more complex tokens are located above the tokens that they use themselves. However, for the rest of the elements it makes sense to put them below the elements they rely on. Therefore, tokens have been moved to the top of the file (since they don't rely on anything else), below that is the XInclude tag, and below that is the rest of the xml file.

Methods

The evaluation of these new tags works by resolving the relative path to an absolute path. It then checks parsed_xmls for an existing XmlParser for that absolute path. If that does not exist, a new XmlParser is created to parse that xml, which adds itself to parsed_xmls once it's done, to prevent regenerating xmls that have already been created. Once that XmlParser exists, all information needed for code generation (versions, tokens, verattrs, path_dict, storage_dict and tag_dict) is copied to the current xml. Information already in the currect xml (the newer one) overwrites information from the older xml. Upon completion of the load_xml function, the XmlParser itself is added to the parsed_xmls dictionary.

Extra checking?

There are two potential problems that could be checked for, if necessary. The current code does not.

  • Redefinition of types. It is possible to redefine types in your xml that were already defined in a included xml file. This was already possible within the same xml file, and was not warned against. There is one slight difference in this behaviour vs if the elements were included directly: if the elements were included directly, their definitions would override existing ones higher up in the xml, which is now not the case.
  • Circular inclusions. Aside from Python's native RecursionError, there are no checks for circular inclusions, so if that happens by mistake, it might take a bit more debugging. I could add it if this is deemed necessary.

f.write(f"{indent}{CONTEXT}.{field_name} = self.{field_name}")
if method_type == 'read':
# store version related stuff on self.context on read
for k, (access, dtype) in self.compound.parser.verattrs.items():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably comment this , maybe pull out into a function too

codegen/expression.py Show resolved Hide resolved
@Candoran2 Candoran2 changed the title Added verattr to xmls and implemented verattr tag codegen. Implementation of verattr tag and xml dependence. Nov 4, 2021
@HENDRIX-ZT2 HENDRIX-ZT2 merged commit a3bae4f into OpenNaja:master Nov 7, 2021
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 this pull request may close these issues.

2 participants