TWKB read and write implementation

[murdos]

I continued previous work done with #452 and twkb branch.

What I've done:

• removed lombok dependency
• moved reader and writer to jts-io-common module, since TWKB is an external format, it's more logical to have it outside jts-core
• split TWKBIO code into TWKBReader and TWKBWriter
• aligned optimizations on PostGis 2.5+ behavior, and removed disabling/enabling optimization from writer API
• regenerated tests data from postgis

[murdos]

@jnh5y : hey! Would you agree to sign the Eclipse agreement with the mail you used in the commits of the twkb branch (jnh5y@****.com)?

[dr-jts]

That's a lot of test data! How long does it take to run? Is there any way to slim it down to focus on just code coverage?

[jnh5y]

@murdos "yes", I'd be happy to sign the commits. That'd change the Git history.

My current ECA is under [email protected]. If you update the author for commits or mention that email things may go better.

FWIW, I'd be fine seeing this all merge-squashed into one commit. I think @jodygarnett would be ok with that as well.

[jnh5y]

Oh, @murdos I answered a different question. I cannot sign the ECA with that email address since I don't work at CCRI anymore. I had that email address when I first starting contributing...

[jnh5y]

Heheh, I agree with Martin, I think we can ditch the massive test files.:)

[murdos]

i've reduced test data to only 78 lines per csv, the whole twkb tests take ~500ms to run.

[murdos]

FWIW, I'd be fine seeing this all merge-squashed into one commit. I think @jodygarnett would be ok with that as well.

Do you mean something like this: https://github.com/locationtech/jts/compare/master...murdos:twkb-squashed?expand=1 ?
It's a bit hard to merge-squashed the whole branch history while keeping commits messages, since master has been merged a few times in that branch.

[dr-jts]

Do you mean something like this: https://github.com/locationtech/jts/compare/master...murdos:twkb-squashed?expand=1 ?

Yup, that's fine. It's going to get squashed on merge anyway.

[jodygarnett]

I think this is okay, see comments below.

@dr-jts do we have any project standards on class javadocs to maintain?

[jodygarnett]

If we are making use of this file as provided, do we wish to also include the test case VarintTest.java?

This header looks fine and amounts to a dependency on Apache mahout version 0.8 (depending on source code rather than binary)

[murdos]

The VarintTest has already been adapted by @groldan : https://github.com/locationtech/jts/pull/854/files#diff-9f94911887b807419c51762589f89bc8ecfdc48eb0b24c01d5919444c3c6acf2

IMO it's useful to have tests that verify behaviour of this utility class, but I'm not opposed to removing it, since it's unlikely that this code will evolve in jts.

[dr-jts]

@dr-jts do we have any project standards on class javadocs to maintain?

There's the CONTRIBUTING page.

Javadoc on classes is always nice, but for a small, clear, purely internal class perhaps it isn't needed.

[groldan]

Hey @murdos I'm so glad to see this work going forward. Unfortunately, I didn't have a chance to continue as it was all in spare time, which has been scarce to be polite. So thanks for picking it up.

[jnh5y]

Would it be possible to document how to use this file?

[murdos]

I added some documentation, please let me know if there's other info you would like to know.

[jnh5y]

@jodygarnett any last minute concerns about the Javadocs? I'm hoping to be free enough on Friday to think about the JTS release.

[murdos]

Hi @dr-jts. Do you have any ETA for the next jts release?

[dr-jts]

Very soon, I hope.