Hi Michaël,
The idea behind openjump-migration is initially to test the SVN to Git
migration and the JTS update. If we start updating some specific code or
creating a test part, it is going to become something else, and it will
be probably difficult to reproduce all these steps when/if you decide to
go on with this migration.
What I was simply suggested was the creation of a dedicated repository
just to test this class (or more if needed), which would benefit from
the JTS update (1.17), not its removal from the OJ core. Simply because
it is the only case which doesn't seem to be easy to solve, and which
requires JTS 1.15+ to be tested against (contrary to all the other
current changes that can be directly committed into the SVN repository,
as usual). Once a solution is found, we can integrate it into the final
migration repository.
So just a simple 'divide and conquer' idea, without the need to
restructure some parts of openjump-migration, at least not for now.
Sorry if I've been unclear.
Eric
On 18/08/2020 09:18, Michaud Michael wrote:
Eric,
Not sure I understand your proposition. There is now a function of
OpenJUMP-core using this MakeValidOp. If we move it to another
project, it will make the build a bit more difficult isn't it ?
About tests, you're completely right. I'm often lazy and just push a
few tests in the main() of the class. I think it would be quite easy
to put these tests in a dedicated test dierctory. Indeed, we already
have one named jumptest, but it is not much used. Benjamin Gudehus, a
former contributor tried to improve our habits about testing, but
there is so much work to be done in this area that nobody followed
him...Let's try to improve it a bit. I'll move MakeValidOp tests to
the dedicated test area.
Michaël
*envoyé :* 18 août 2020 à 09:55
*de :* Eric <eric.openj...@thefactory.io>
*à :* jump-pilot-devel@lists.sourceforge.net
*objet :* Re: [JPP-Devel] MakeValidOp
Hi Michaël,
Don't be sorry, it was quite good to understand how it works.
The changes I made seem to fix the part of makeValid which tests if
the "geometry" dimension (based on CoordinateSequence, not the
geometry dimension itself). If the dimension is 4, it calls
restoreDim4 as expected. These shouldn't have an impact on the rest.
The other changes are related to the PackedCoordinateSequenceFactory
instances, as there isn't any more a constructor which includes the
dimension as a parameter.
As it purely JTS dependent, would you like to create another
repository for that in openjump-gis or would you like me to do it?
This way we could split the code and the tests, and it would be
easier to manage than the full project (especially if the tests are
written separately, as it isn't yet clearly done in OJ at the
moment). Let me know what you think about it.
Eric
On 18/08/2020 07:41, Michaud Michael wrote:
Hi Eric,
Sorry to have let you with this problem.
I also checked on my side. Indeed, when I wrote this class, I gave
up the idea of preserving the z/m ordinate during the computation
because as you noticed, they are lost by some jts algo I need.
Instead, I get z/m back at the end of the process.
I think that broken tests are not so important because they test
intermediate steps (on CoordinateSequence). I suppose they are
broken by the many changes done on jts side about z/m management.
Trying to understand why these changes broke my test, I was not
completely satisfied about how this is managed in jts and I issued 2
tickets in locationtech/jts repo.
-> I have a version of the class which compiles and passed the
(modified) tests. I can push it.
-> I'll review the code depending on how my issues on jts are
answered. I also have to check it more extensively to take into
account improvements on z/m management recently done in jts. But it
can be done later.
Michaël
*envoyé :* 18 août 2020 à 04:29
*de :* Eric <eric.openj...@thefactory.io>
<mailto:eric.openj...@thefactory.io>
*à :* jump-pilot-devel@lists.sourceforge.net
<mailto:jump-pilot-devel@lists.sourceforge.net>
*objet :* Re: [JPP-Devel] Git migration
Hi,
No problem.
I didn't encounter any problems to complete the migration locally
(removed WFS parts, updated related WFS classes, JTS 1.17 and
related code updated, etc.)... except with the class
'com.vividsolutions.jump.geom.MakeValidOp'.
I managed to modify/update this class and it compiles. But then I
tried to test if it was working based on the tests already written
in the main. Even if the tests can be run, it seems that there is a
problem with the 'nodePolygon(Polygon polygon)' method, during the
validation of 3D (but also 4D) geometries (i.e. XYZ and XYZM). It
replaces the Z values by NaN, thus changing 3D coordinate sequences
into 2D ones. And because the mapping of the measure (M) is based
on a XYZ coordinate comparison, this measure is also lost for 4D
geometries. I didn't test yet this method with the current OJ
version, to see if it is linked to the JTS update or not.
I don't think that I can quickly solve it. So what I would suggest
for now is to leave this problem aside. Then tomorrow / today, I'll
remove the extra code I wrote to locate/fix this possible bug, and
I'll push the changes I made locally. This way, you'll be able to
access the OJ version (with JTS 1.17 but without WFS). If I have
time, I'll also try to add a first script to automate the builds,
probably using Travis.
Sorry to not have been able to push my local changes today /
yesterday but I really tried to see what was happening with the
MakeValidOp app.
For info, I also documented the changes I made during this second
phase and how I made them (even if the results / diffs can be
visible in the commits / logs). I'll add both documentation (svn to
git migration and this one) as soon as it is ready.
Eric
On 15/08/2020 17:27, Michaud Michael wrote:
Thanks for you effort,
I pushed a small modification in the pom to test that I can access
and compile the project. Could compile after that.
Was just a test, don't hesitate if you have to restart the process
from scratch.
Michaël
envoyé : 15 août 2020 à 12:36
de : Eric <eric.openj...@thefactory.io>
<mailto:eric.openj...@thefactory.io>
à : jump-pilot-devel@lists.sourceforge.net
<mailto:jump-pilot-devel@lists.sourceforge.net>
objet : Re: [JPP-Devel] Git migration
Hi Ede,
On 15/08/2020 11:19, edgar.sol...@web.de
<mailto:edgar.sol...@web.de> wrote:
On 15.08.2020 12:07, Eric wrote:
>> Hi all,>>
>> After 5-6 hours, the result of the migration is finally
complete: https://github.com/openjump-gis/openjump-migration
>>
>> It includes the commit history from revision 859 to 6242.
just checked, we'll lose some commits to the source this way.
did you check if you could svn2git from rev.1 ? did it err out?
I'll try it next time from revision 1 as it would be a bit long
to do it
again just now (it needs to run at night), and unnecessary within
the
context of the JTS update.
>> I reinitialised the repository 'openjump-migration' to make
easier this initial import, i.e. I deleted it and recreated a new
one with the same name, but empty this time.
>>
>> I'm now going to add a couple of files (gitignore, licence,
readme, etc.) then I'll delete the WFS part as discussed, update
the Maven configuration, update JTS and all related classes.
sounds good. remember to write down the steps, so we can
recreate it in case we want/need to.
Don't worry, it is well documented. It also explains the reasons
behind
some of the choices.
I just added a first gitignore file and the licence. I'm writing a
readme right now, then I'll convert the documentation about the
migration from txt to md. One step at a time.
Eric
thanks ..ede
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
<mailto:Jump-pilot-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
<mailto:Jump-pilot-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
<mailto:Jump-pilot-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
<mailto:Jump-pilot-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
<mailto:Jump-pilot-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
