orbisgis / cts

Projection library in Java
GNU Lesser General Public License v3.0
50 stars 16 forks source link

Javadoc + code simplification + #59

Closed mukoki closed 7 years ago

mukoki commented 10 years ago

Method signature simplified in CoordinateOperationFactory

mukoki commented 10 years ago

I tried to make CRSFactory / Registry more format-independant

The main change is in the Registry interface definition : public String getRegistryName(); ok // getParameters(String code) replaced by public CoordinateReferenceSystem getCoordinateReferenceSystem(Identifier id); public Set getSupportedCodes() throws RegistryException; ok

Registry should be able to manage CRS from

Beside Registry interface, the main change is in AbstractProjRegistry which now implements getCoordinateReferenceSystem(Identifier) using CRSHelper.

Not sure I have seen all the inbrications, but I think the api is a bit more flexible like this.

ebocher commented 10 years ago

Could please update the cts version in the pom ?

nicolas-f commented 10 years ago

CTS is in snapshot, there is no need to change the version

ebocher commented 10 years ago

Thanks a lot @mukoki. I will look.

nicolas-f commented 9 years ago

We cannot accept because there is conflict with current master branch. Please do a merge fix on your side.

ebocher commented 9 years ago

@nicolas-f I will do it.

ebocher commented 9 years ago

The merge is planned before the end of February. Sorry for the delay.

ebocher commented 9 years ago

@mukoki . I' m not able to compile this PR due to a bad class name. worldRegistry must be replaced by WorldRegistry. Anyway, org.cts.op.CirceMetropoleComplianceTest class contains 4 assertion errors. So I cannot merge before you fixe them.

mukoki commented 9 years ago

Hi Erwan,

I just made some more fixes in CTS today and pushed them from my local project. (I don't remember if I must also create a PullRequest from Github - let me know). I have changed worldRegistry to WorldRegistry in my code a long time ago. I don't know why you don't get it. About tests which do not pass, I made some fixes and deactivated a test on New Caledonia (I wrote a comment about this test).

Best regards,

Michaël

@mukoki https://github.com/mukoki . I' m not able to compile this PR due to a bad class name. worldRegistry must be replaced by WorldRegistry. Anyway, org.cts.op.CirceMetropoleComplianceTest class contains 4 assertion errors. So I cannot merge before you fixe them.

— Reply to this email directly or view it on GitHub https://github.com/irstv/CTS/pull/59#issuecomment-72430224.

ebocher commented 9 years ago

Dear @mukoki ,

Thanks a lot but I m not able to build the last PR. 4 tests are in errors.

testLAMBE_to_ETRS_LCC(org.cts.op.CirceMetropoleComplianceTest)  Time elapsed: 0.021 sec  <<< FAILURE!
java.lang.AssertionError: null
    at org.junit.Assert.fail(Assert.java:92)
    at org.junit.Assert.assertTrue(Assert.java:43)
    at org.junit.Assert.assertTrue(Assert.java:54)
    at org.cts.op.BaseCoordinateTransformTest.test(BaseCoordinateTransformTest.java:136)
    at org.cts.op.CirceMetropoleComplianceTest.testLAMBE_to_ETRS_LCC(CirceMetropoleComplianceTest.java:359)

testLAMBE_to_ETRS_LAEA(org.cts.op.CirceMetropoleComplianceTest)  Time elapsed: 0.041 sec  <<< FAILURE!
java.lang.AssertionError: null
    at org.junit.Assert.fail(Assert.java:92)
    at org.junit.Assert.assertTrue(Assert.java:43)
    at org.junit.Assert.assertTrue(Assert.java:54)
    at org.cts.op.BaseCoordinateTransformTest.test(BaseCoordinateTransformTest.java:136)
    at org.cts.op.CirceMetropoleComplianceTest.testLAMBE_to_ETRS_LAEA(CirceMetropoleComplianceTest.java:364)

testLAMBE_to_ETRS_UTM32N(org.cts.op.CirceMetropoleComplianceTest)  Time elapsed: 0.02 sec  <<< FAILURE!
java.lang.AssertionError: null
    at org.junit.Assert.fail(Assert.java:92)
    at org.junit.Assert.assertTrue(Assert.java:43)
    at org.junit.Assert.assertTrue(Assert.java:54)
    at org.cts.op.BaseCoordinateTransformTest.test(BaseCoordinateTransformTest.java:136)
    at org.cts.op.CirceMetropoleComplianceTest.testLAMBE_to_ETRS_UTM32N(CirceMetropoleComplianceTest.java:369)

testNTF_P_GEO2D_EGM2008_To_WGS84_C(org.cts.op.CirceMetropoleComplianceTest)  Time elapsed: 0.003 sec  <<< FAILURE!
java.lang.AssertionError: null
    at org.junit.Assert.fail(Assert.java:92)
    at org.junit.Assert.assertTrue(Assert.java:43)
    at org.junit.Assert.assertTrue(Assert.java:54)
    at org.cts.op.BaseCoordinateTransformTest.test(BaseCoordinateTransformTest.java:132)
    at org.cts.op.CirceMetropoleComplianceTest.testNTF_P_GEO2D_EGM2008_To_WGS84_C(CirceMetropoleComplianceTest.java:247)
ebocher commented 9 years ago

dear @mukoki . Any input about this errors.

mukoki commented 9 years ago

WorldRegistry typo is resolved on my side. I deactivated some test on new caledonia (previous pull request). It failed, but it is not clear for me which algo / parameters is used by circe. One test failed because egm08fr.mnt was missing (it is quite a big file !). I added it to the repository. I've now 231/231 tests passing. I'm not sure I solved all the failure you get, but I have no more on my machine. Note : a few months ago, I observed that results were varying depending on the jvm (7 vs 8) ! It was small variations and I don't think it can make tests fail. But I was surprised that double calculations is not perfectly predictable.

ebocher commented 9 years ago

I will look on it asap. About the new grid file, maybe we have to think about an extension mechanism ? Otherwise the CTS binary distribution will be too big.

nicolas-f commented 9 years ago

For the next minor release, we could create another optional maven projects for each grid based registry collection. Something like cts-epsg-egm. What do you think ?

ebocher commented 8 years ago

This integration is planed before the end of the year. Some refactoring will be done to manage grid file dependencies.

mukoki commented 8 years ago

I fixed a bug in CoordinateSequence simplification mechanism, improved Projection.inverse operations, improved/add comments on CompoundCRS and add some finnish projection. This version may not be compatible with the previous one.

ebocher commented 8 years ago

Thanks a lot. I will look on it asap. We are a bit busy since the new team location in Vannes.

ebocher commented 8 years ago

We must find a solution to manage the egm08fr.mnt because this file is too big.

ebocher commented 8 years ago

Please could update the pr to resolve the conflicts

ebocher commented 7 years ago

c