[1.3] Version handling, parser fixes, setup update #197

mpsonntag · 2017-12-07T15:44:15Z

This PR introduces the following changes to the 1.3 branch:

[Doc format] Provides a package and a document format version number in the new file info.py
[Doc format] All parsers now use the common document format version number.
[Doc format] When trying to open a file with any of the odml parsers, first the document format version number is checked. If the found version number does not match the supported one, file loading will fail an exception, since this is the oldest format version. If anyone tries to open a newer format, they should first update their odml package and not use this one...
[Doc saving] An odML document can now only be saved, if the validation does not show any errors. Saving an invalid document will exit while printing all encountered errors.
[Parser loading] All parsers are now more relaxed when encountering unsupported tags or missing tags and only print warnings instead of ending with an exception. Warnings are collected and can be accessed via the parser object (required for display in odml-ui to avoid potential loss of information).
[Setup] now contains updated authors, contact, website, license and classifiers.
Cherry picks from the development branch to address fileio/odmlparser error when loading JSON or YAML #191 and odmlparser error on section attributes #194 also in the 1.3 branch.

Added author list, contact, classifiers, license text and updated the provided hompage for future packaging.

For now when xmlparser is used from odmlparser, all unsupported or missing element errors are warnings to allow parsing of partially invalid odML documents.

coveralls · 2017-12-07T16:08:31Z

Coverage increased (+0.1%) to 80.419% when pulling 64a91d5 on mpsonntag:picks into fea65ed on G-Node:version_1.3.

jgrewe

lgtm

coveralls · 2017-12-11T14:03:27Z

Coverage increased (+0.1%) to 80.419% when pulling 2415055 on mpsonntag:picks into fea65ed on G-Node:version_1.3.

coveralls · 2017-12-12T15:42:06Z

Coverage increased (+0.1%) to 80.419% when pulling c8e6157 on mpsonntag:picks into fea65ed on G-Node:version_1.3.

achilleas-k

Looks good. Couldn't find any serious issues, so had to nitpick typos instead.

The import cycle issue can be fixed in a later PR since it will require a bit more work than just moving around a file.

achilleas-k · 2017-12-12T13:53:57Z

odml/tools/xmlparser.py

+        """
+        Check if the odML version of a handed in parsed lxml.etree is supported
+        by the current library and raise an Exception otherwise.
+        :param root: Root node of a parsed lxml.etree. Teh root tag has to


While I appreciate a nice "Teh", I think in this case it's an actual typo.

achilleas-k · 2017-12-12T13:56:26Z

odml/tools/xmlparser.py

+                     accepted as a valid odML file.
+        """
+        if root.tag != 'odML':
+            raise ParserException("Expecting <odML> start tag but got <%s>.\n" % root.tag)


Should we call it an open instead of a start tag? I think people tend to say "open a X tag". Or maybe we can say neither and just have it say "Expecting tag but got ".

achilleas-k · 2017-12-12T13:56:28Z

odml/tools/xmlparser.py

+            raise ParserException("Expecting <odML> start tag but got <%s>.\n" % root.tag)
+        elif 'version' not in root.attrib:
+            raise ParserException("Could not find format version attribute "
+                                  "in odML start tag.\n")


Same as above.

achilleas-k · 2017-12-12T15:34:33Z

odml/tools/odmlparser.py

@@ -122,6 +122,17 @@ def get_values(self, value_list):
        return value_seq

    def write_file(self, odml_document, filename):
+        # Write document only if it does not contain validation errors.
+        from ..validation import Validation  # disgusting import problems


I tried to shuffle things around a bit to make this a bit more sane. It's possible to avoid this import cycle if we move the validation file to the tools submodule, but I think we have bigger import issues. One issue is we don't use explicit relative imports everywhere. This might lead to issues when testing local code while a global version of the package is installed. It might also lead to subtle bugs where local file location are changed, but the absolute import hides this by importing from an installed version.

This leads me to a different issue I came across. We have a lot of functions in the top-level __init__.py. This isn't a problem in and of itself, but we do import one of these functions into the validation file, which makes internal package imports impossible. So we end up with a situation where the validation file imports odml, which relies on importing everything else, which obviously would result in an import cycle.

coveralls · 2017-12-12T15:50:10Z

Coverage increased (+0.1%) to 80.419% when pulling c2ae4b5 on mpsonntag:picks into fea65ed on G-Node:version_1.3.

mpsonntag added 19 commits December 6, 2017 15:46

[section] Add missing attributes to init

d0fa7ee

[tools/odmlparser] Fix open JSON file bug

3e83205

[tools/odmlparser] Fix open YAML file bug

cf60c47

Closes G-Node#191.

[tools/odmlparser] Fix YAML/JSON Reader.from_str

52b094f

[test/parser] Fix open YAML/JSON file bug

ea3f263

[tools/odmlparser] Pep8 cleanup

e2be96f

[init] Update version number

d2550f6

[info] Add info file to provide version and format

00c995b

[init] User version number from info

88fc8c9

[odmlparser] Use format version number from info

1820178

[xmlparser] Use format version from info

7d61eac

[xmlparser] Add odml format version check

9e0c50a

[odmlparser] Add odml format version check

713e720

[init/setup] Update packaging information

c9c4dd6

Added author list, contact, classifiers, license text and updated the provided hompage for future packaging.

[odmlparser] Save only valid odML documents

577b8ff

[odmlparser] Use xmlparser with ignore_errors

9b7baac

For now when xmlparser is used from odmlparser, all unsupported or missing element errors are warnings to allow parsing of partially invalid odML documents.

[xmlparser] Add parser warning list

5f29fcd

[odmlparser] Add parser warning list

5d2701b

[travis] Use master branch travis file

64a91d5

jgrewe approved these changes Dec 9, 2017

View reviewed changes

[parser] Update invalid odML format version text

2415055

mpsonntag added 2 commits December 12, 2017 15:43

[fileio] Define parsers only in odmlparser

f45e9bc

[fileio] Add docstrings

c8e6157

achilleas-k approved these changes Dec 12, 2017

View reviewed changes

[xmlparser] Fixing typos

c2ae4b5

achilleas-k merged commit bf88a3c into G-Node:version_1.3 Dec 12, 2017

mpsonntag deleted the picks branch December 12, 2017 16:15

This was referenced Dec 12, 2017

[1.4] Version handling, parser fixes, setup update, fix terminology load #198

Merged

Check format version on open/load #180

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1.3] Version handling, parser fixes, setup update #197

[1.3] Version handling, parser fixes, setup update #197

mpsonntag commented Dec 7, 2017

coveralls commented Dec 7, 2017

jgrewe left a comment

coveralls commented Dec 11, 2017

coveralls commented Dec 12, 2017

achilleas-k left a comment

achilleas-k Dec 12, 2017

achilleas-k Dec 12, 2017

achilleas-k Dec 12, 2017

achilleas-k Dec 12, 2017

coveralls commented Dec 12, 2017

[1.3] Version handling, parser fixes, setup update #197

[1.3] Version handling, parser fixes, setup update #197

Conversation

mpsonntag commented Dec 7, 2017

coveralls commented Dec 7, 2017

jgrewe left a comment

Choose a reason for hiding this comment

coveralls commented Dec 11, 2017

coveralls commented Dec 12, 2017

achilleas-k left a comment

Choose a reason for hiding this comment

achilleas-k Dec 12, 2017

Choose a reason for hiding this comment

achilleas-k Dec 12, 2017

Choose a reason for hiding this comment

achilleas-k Dec 12, 2017

Choose a reason for hiding this comment

achilleas-k Dec 12, 2017

Choose a reason for hiding this comment

coveralls commented Dec 12, 2017