Add LandXML support

[psolyca]

Add support of LandXML as input and output.

[steko]

This is a lot of work! It's fantastic!!!

It was not clear from the description that you were adding both input and output support for the format 👍

The biggest issue I can find - and that's not very big! - is the lack of documentation, both in general and for specific details, e.g. the different values for processing points "PO", "PT" etc. is super efficient but it must be explained somewhere in the user documentation.

There's also some confusion with the tests, since only the output is covered. The existing tests for other formats use this simplistic procedure of "sampling" some points but I think it's better to use a complete sample file (or more than one). Sample files should go to sample_data and can be used for tests (in this case, both for input and output).

I have added more comments on small details in some files.

Thank you! 😁

[steko]

OK I'm just asking for a few small changes that I didn't see yesterday (file names, missing test coverage).

And the documentation: if you want I can start writing it?

[psolyca]

I will do it.

[psolyca]

After rereading the code, PO, PT and ST are values used internally and not provided to end users, thus I will had a page (or multiple) on explanation around Tops development.
As these values have been introduced long time ago, I will not add them on this PR but will create another one.

[psolyca]

I need some changes in the code before releasing this parser.

[steko]

Do you need any help to update this pull request?

Tests are failing because some of our input formats have no Z coordinate and the LandXML output module doesn't handle the case of missing Z coordinate:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
totalopenstation/output/tops_landxml.py:53: in process
    kwargs = self._get_feature(feature)
totalopenstation/output/tops_landxml.py:41: in _get_feature
    kwargs["z"] = feature.geometry.z
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = Point(0.0, 0.0)
    @property
    def z(self):
        """Return z coordinate."""
        if len(self._coordinates) != 3:
>           raise ValueError("This point has no z coordinate.")
E           ValueError: This point has no z coordinate.

[psolyca]

I will look at this after merging master.
I will have lots of work this end of year. I hope to be able to finish this for end of December but I can not guarantee.

[steko]

Take your time, there is no deadline.

[psolyca]

Weird results of test cases https://github.com/totalopenstation/totalopenstation/runs/1407259466 ^^;
Values are correct but not in the right order for 3.6, 3.7 and pypy3.
It could be a "problem" of order in dict but it has been introduced in 3.7 and 3.7 is not correct.
So should we pass through this ?

[steko]

Oh, that's LandXML support coming! 😄

About those weird test results, I'm not sure it depends on ordered dictionary since you're testing the output of tostring(). Since I don't like having test failures around too much, I propose the following workaround.

Replace these two lines

        self.assertEqual(self.output.splitlines()[12], b'\t\t<InstrumentSetup id="setup0" stationName="STATION" instrumentHeight="1.5">')
        self.assertEqual(self.output.splitlines()[16], b'\t\t\t<RawObservation targetHeight="1.5" horizAngle="0" zenithAngle="90.585" slopeDistance="1718.28">')

with

        self.assertIn(b'<InstrumentSetup', self.output.splitlines()[12])
        self.assertIn(b'id="setup0"', self.output.splitlines()[12])
        self.assertIn(b'instrumentHeight="1.5"', self.output.splitlines()[12])
        self.assertIn(b'stationName="STATION"', self.output.splitlines()[12])
        self.assertIn(b'<RawObservation', self.output.splitlines()[16])
        self.assertIn(b'targetHeight="1.5"', self.output.splitlines()[16])
        self.assertIn(b'horizAngle="0"', self.output.splitlines()[16])
        self.assertIn(b'zenithAngle="90.585"', self.output.splitlines()[16])
        self.assertIn(b'slopeDistance="1718.28"', self.output.splitlines()[16])

so the order doesn't matter. After all we're not testing that XML is well formed (we trust the standard library to do that).

I think that settles the problem and makes LandXML a first class format 🚀

[psolyca]

Thanks !
You were faster than me ^^

[steko]

Huge work!
I think one of the Travis checks will hang forever since it's not a branch in this repo (we may remove the check altogether).
I'll just merge this. There's still some documentation missing but it can be done in a smaller PR.

[psolyca]

I was looking why the check did not pass but ok to merge.
I will do a PR for the documentation later.