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

Nxquadric 1 #627

Merged
merged 8 commits into from
Oct 24, 2018
Merged

Nxquadric 1 #627

merged 8 commits into from
Oct 24, 2018

Conversation

tcspain
Copy link
Contributor

@tcspain tcspain commented Sep 27, 2018

Proposed NXquadric and NXcsg classes to allow the definition of constructive solid geometry from quadric surfaces.

NXDL description XML files for the three NXquadric and constructive solid geometry classes.
Delete the dummy file
@zjttoefs
Copy link
Contributor

That build failed, please fix the tags.

@prjemian
Copy link
Contributor

Please remove the example data from this PR. You are encouraged to make a companion PR in the NeXus repository for example data: https://github.com/nexusformat/exampledata

You may need to create a subdirectory path for this. Try to follow the apparent practice. Add a README.md with the example data to describe the file(s) such as what it is and whether or not the files are compliant with the NeXus standard (version ____).

@prjemian prjemian self-requested a review September 27, 2018 17:25
@prjemian
Copy link
Contributor

prjemian commented Sep 27, 2018

The failure is in the XML:

======================================================================
ERROR: test__3__contributed_definitions__NXcsg (test_nxdl.Individual_NXDL_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/nexusformat/definitions/utils/test_nxdl.py", line 90, in test_wrap
    validate_xml(nxdl_file_name)
  File "/home/travis/build/nexusformat/definitions/utils/test_nxdl.py", line 54, in validate_xml
    raise NXDL_Invalid(msg)
NXDL_Invalid: contributed_definitions/NXcsg.nxdl.xml : error parsing attribute name, line 38, column 5 (line 38)
======================================================================
ERROR: test__3__contributed_definitions__NXquadric (test_nxdl.Individual_NXDL_Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/nexusformat/definitions/utils/test_nxdl.py", line 90, in test_wrap
    validate_xml(nxdl_file_name)
  File "/home/travis/build/nexusformat/definitions/utils/test_nxdl.py", line 54, in validate_xml
    raise NXDL_Invalid(msg)
NXDL_Invalid: contributed_definitions/NXquadric.nxdl.xml : Opening and ending tag mismatch: field line 49 and definition, line 87, column 14 (line 87)
  1. In NXcsg.nxdl.xml, line 37, you are missing the closing >
  2. In NXquadric.nxdl.xml, after line 76 (before line 77), you are missing the closing </field> that opened on line 49.
  3. In NXquadric.nxdl.xml, line 50, the field name type could be more specific in my opinion. Since type is common in XML files and other contexts, confusion of this generic name with some other concept is possible.

Copy link
Contributor

@prjemian prjemian left a comment

Choose a reason for hiding this comment

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

commented previously, changes made, passes travis-ci validation checks, ok. Thanks!

@prjemian
Copy link
Contributor

prjemian commented Oct 3, 2018

Comments by anyone else? Shall we merge? Marking for next telco.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants