This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 91332ab Investigate why some CRS are considered not equal when they should be. For now we just documented the finding in the following JIRA task; no fix has been attempted yet: - https://issues.apache.org/jira/browse/SIS-433 - https://issues.apache.org/jira/browse/SIS-434 91332ab is described below commit 91332abc3666b12b26af5731eb89314380071686 Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Wed Oct 17 13:15:26 2018 +0200 Investigate why some CRS are considered not equal when they should be. For now we just documented the finding in the following JIRA task; no fix has been attempted yet: - https://issues.apache.org/jira/browse/SIS-433 - https://issues.apache.org/jira/browse/SIS-434 --- application/sis-openoffice/pom.xml | 6 +++ .../org/apache/sis/referencing/cs/AbstractCS.java | 8 ++- .../operation/AbstractCoordinateOperation.java | 5 +- .../operation/projection/ObliqueMercator.java | 2 +- .../sis/test/integration/ConsistencyTest.java | 63 +++++++++++++++++----- .../main/java/org/apache/sis/measure/Units.java | 2 +- .../main/java/org/apache/sis/util/Utilities.java | 8 +-- pom.xml | 15 +++++- 8 files changed, 82 insertions(+), 27 deletions(-) diff --git a/application/sis-openoffice/pom.xml b/application/sis-openoffice/pom.xml index 9fcfdd9..35dd9b1 100644 --- a/application/sis-openoffice/pom.xml +++ b/application/sis-openoffice/pom.xml @@ -142,6 +142,12 @@ <artifactId>sis-embedded-data</artifactId> <version>${sis.non-free.version}</version> <scope>runtime</scope> + <exclusions> + <exclusion> + <groupId>*</groupId> + <artifactId>*</artifactId> + </exclusion> + </exclusions> </dependency> </dependencies> </profile> diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java index eb33e8c..35a749e 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/cs/AbstractCS.java @@ -406,11 +406,9 @@ public class AbstractCS extends AbstractIdentifiedObject implements CoordinateSy return Arrays.equals(axes, ((AbstractCS) object).axes); } case DEBUG: { - final int d1 = axes.length; - final int d2 = ((CoordinateSystem) object).getDimension(); - if (d1 != d2) { - throw new AssertionError(Errors.format(Errors.Keys.MismatchedDimension_2, d1, d2)); - } + final int d1, d2; + assert (d1 = axes.length) == (d2 = ((CoordinateSystem) object).getDimension()) + : Errors.format(Errors.Keys.MismatchedDimension_2, d1, d2); // Fall through } default: { diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java index 6d7aaa5..c421576 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/AbstractCoordinateOperation.java @@ -868,8 +868,10 @@ check: for (int isTarget=0; ; isTarget++) { // 0 == source check; 1 * this.sourceCRS == AbstractDerivedCRS.baseCRS. Consequently we can relax the check of * sourceCRS axis order if the mode is ComparisonMode.IGNORE_METADATA. */ + boolean debug = false; if (Semaphores.queryAndSet(Semaphores.CONVERSION_AND_CRS)) { if (mode.isIgnoringMetadata()) { + debug = (mode == ComparisonMode.DEBUG); mode = ComparisonMode.ALLOW_VARIANT; } } else try { @@ -902,7 +904,8 @@ check: for (int isTarget=0; ; isTarget++) { // 0 == source check; 1 Logging.recoverableException(Logging.getLogger(Loggers.COORDINATE_OPERATION), AbstractCoordinateOperation.class, "equals", e); } - return deepEquals(tr1, tr2, mode); + if (deepEquals(tr1, tr2, mode)) return true; + assert !debug || deepEquals(tr1, tr2, ComparisonMode.DEBUG); // For locating the mismatch. } } } diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/projection/ObliqueMercator.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/projection/ObliqueMercator.java index aa5e48e..742461d 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/projection/ObliqueMercator.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/projection/ObliqueMercator.java @@ -241,7 +241,7 @@ public class ObliqueMercator extends ConformalProjection { * At this point, all parameters have been processed. Now process to their * validation and the initialization of (de)normalize affine transforms. */ - getContextualParameters().getMatrix(MatrixRole.NORMALIZATION).convertAfter(0, null, -λ0); + context.getMatrix(MatrixRole.NORMALIZATION).convertAfter(0, null, -λ0); final MatrixSIS denormalize = getContextualParameters().getMatrix(MatrixRole.DENORMALIZATION); final Matrix3 rotation = new Matrix3(); rotation.m00 = rotation.m11 = cos(γc); diff --git a/core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java b/core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java index d2a7eef..c364c7f 100644 --- a/core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java +++ b/core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java @@ -20,6 +20,9 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.text.ParseException; +import javax.measure.Quantity; +import javax.measure.Unit; +import javax.measure.UnitConverter; import org.opengis.metadata.Identifier; import org.opengis.util.FactoryException; import org.opengis.util.NoSuchIdentifierException; @@ -55,7 +58,7 @@ import static org.junit.Assume.assumeTrue; * This test is executed only if {@link #RUN_EXTENSIVE_TESTS} is {@code true}. * * @author Martin Desruisseaux (Geomatys) - * @version 0.8 + * @version 1.0 * @since 0.7 * @module */ @@ -84,6 +87,24 @@ public final strictfp class ConsistencyTest extends TestCase { private int codeWidth = 15; /** + * Specialization of {@link #testCoordinateReferenceSystems()} for specific cases that were known to fail. + * This is used for debugging purposes only; not included in normal test execution because it is redundant + * with {@link #testCoordinateReferenceSystems()}. + * + * @throws FactoryException if the coordinate reference system can not be created. + * + * @see <a href="https://issues.apache.org/jira/browse/SIS-433">SIS-433</a> + * @see <a href="https://issues.apache.org/jira/browse/SIS-434">SIS-434</a> + */ + public void debug() throws FactoryException { + final String code = "EPSG::29871"; + final CoordinateReferenceSystem crs = CRS.forCode(code); + final WKTFormat format = new WKTFormat(null, null); + format.setConvention(Convention.WKT2); + lookup(parseAndFormat(format, code, crs), crs); + } + + /** * Verifies the WKT consistency of all CRS instances. * * @throws FactoryException if an error other than "unsupported operation method" occurred. @@ -100,7 +121,7 @@ public final strictfp class ConsistencyTest extends TestCase { v2 .setConvention(Convention.WKT2); v2s.setConvention(Convention.WKT2_SIMPLIFIED); for (final String code : CRS.getAuthorityFactory(null).getAuthorityCodes(CoordinateReferenceSystem.class)) { - if (!EXCLUDES.contains(code)) { + if (!EXCLUDES.contains(code) && !code.startsWith("Proj4:")) { final CoordinateReferenceSystem crs; try { crs = CRS.forCode(code); @@ -234,21 +255,37 @@ public final strictfp class ConsistencyTest extends TestCase { /** * Verifies that {@code IdentifiedObjects.lookupURN(…)} on the parsed CRS can find back the original CRS. */ - private static void lookup(final CoordinateReferenceSystem parsed, final CoordinateReferenceSystem crs) - throws FactoryException - { + private void lookup(final CoordinateReferenceSystem parsed, final CoordinateReferenceSystem crs) throws FactoryException { final Identifier id = IdentifiedObjects.getIdentifier(crs, null); - /* - * Lookup operation is not going to work if the CRS are not approximately equal. - */ final String urn = IdentifiedObjects.toURN(crs.getClass(), id); assertNotNull(crs.getName().getCode(), urn); - assertTrue(urn, Utilities.deepEquals(crs, parsed, ComparisonMode.DEBUG)); /* - * Now test the lookup operation. Since the parsed CRS has an identifier, - * that lookup operation should not do a lot of work actually. + * Lookup operation is not going to work if the CRS are not approximately equal. + * However in current Apache SIS implementation, we can perform this check only + * if the scale factor of units of measurement have the exact same value. + * + * This check can be removed after the following issue is resolved: + * https://issues.apache.org/jira/browse/SIS-433 */ - final String lookup = IdentifiedObjects.lookupURN(parsed, null); - assertEquals("Failed to lookup the parsed CRS.", urn, lookup); + if (toStandardUnit(crs .getCoordinateSystem().getAxis(0).getUnit()).equals( + toStandardUnit(parsed.getCoordinateSystem().getAxis(0).getUnit()))) + { + assertTrue(urn, Utilities.deepEquals(crs, parsed, ComparisonMode.DEBUG)); + /* + * Now test the lookup operation. Since the parsed CRS has an identifier, + * that lookup operation should not do a lot of work actually. + */ + final String lookup = IdentifiedObjects.lookupURN(parsed, null); + assertEquals("Failed to lookup the parsed CRS.", urn, lookup); + } else { + print(id.getCode(), "SKIPPED", "Unit conversion factors differ."); + } + } + + /** + * Returns the converter to standard unit. + */ + private static <Q extends Quantity<Q>> UnitConverter toStandardUnit(final Unit<Q> unit) { + return unit.getConverterTo(unit.getSystemUnit()); } } diff --git a/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java b/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java index 48ceabd..5bfde64 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java +++ b/core/sis-utility/src/main/java/org/apache/sis/measure/Units.java @@ -1619,7 +1619,7 @@ public final class Units extends Static { * for fetching the base units, and derives automatically other units from the information * found in the EPSG database. This method is also used by other classes not directly related * to the EPSG database, like {@link org.apache.sis.referencing.factory.CommonAuthorityFactory} - * which uses EPSG code for identifying units.</p> + * which uses EPSG codes for identifying units.</p> * * <p>The currently recognized values are:</p> * <table class="sis"> diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/Utilities.java b/core/sis-utility/src/main/java/org/apache/sis/util/Utilities.java index 7d9b6a1..19b6fc2 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/util/Utilities.java +++ b/core/sis-utility/src/main/java/org/apache/sis/util/Utilities.java @@ -229,7 +229,7 @@ public final class Utilities extends Static { final Iterator<?> it2 = object2.iterator(); while (it1.hasNext()) { if (!it2.hasNext()) { - assert isNotDebug(mode) : mismatchedElement("Iterable", object1, object2, "size"); + assert isNotDebug(mode) : mismatchedElement("Iterable", object1, object2, "sizes"); return false; } Object element1 = it1.next(); @@ -238,7 +238,7 @@ public final class Utilities extends Static { continue; } if (!(object1 instanceof Set<?> && object2 instanceof Set<?>)) { - assert isNotDebug(mode) : mismatchedElement("Iterable", object1, object2, "element"); + assert isNotDebug(mode) : mismatchedElement("Iterable", object1, object2, "elements"); return false; } /* @@ -258,7 +258,7 @@ public final class Utilities extends Static { while (true) { final Iterator<?> it = copy.iterator(); do if (!it.hasNext()) { - assert isNotDebug(mode) : mismatchedElement("Set", object1, object2, "element"); + assert isNotDebug(mode) : mismatchedElement("Set", object1, object2, "elements"); return false; // An element has not been found. } while (!deepEquals(it.next(), element2, mode)); it.remove(); @@ -303,7 +303,7 @@ public final class Utilities extends Static { if (type == null && object2 instanceof CheckedContainer<?>) { type = ((CheckedContainer<?>) object2).getElementType(); } - return header + '<' + (type != null ? type.getSimpleName() : "?") + ">: " + tail + " not equals."; + return header + '<' + (type != null ? type.getSimpleName() : "?") + ">: " + tail + " not equal."; } /** diff --git a/pom.xml b/pom.xml index 4961239..c6de25f 100644 --- a/pom.xml +++ b/pom.xml @@ -19,6 +19,17 @@ under the License. --> +<!-- ============================================================================================= + Maven 2 project configuration file + http://maven.apache.org/maven2/ + + Apache SIS build requires Java 10, but compilation result can be executed on Java 8. + Setting the SIS_DATA environment variable before build is optional but recommended. + + Build development snapshot: mvn clean install + Include EPSG database: mvn install -Pnon-free + Run more extensive tests: mvn test -Dorg.apache.sis.test.extensive=true + ============================================================================================= --> <project xmlns = "http://maven.apache.org/POM/4.0.0" xmlns:xsi = "http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation = "http://maven.apache.org/POM/4.0.0 @@ -513,7 +524,7 @@ <maven.compiler.source>8</maven.compiler.source> <maven.compiler.target>8</maven.compiler.target> <sis.plugin.version>${project.version}</sis.plugin.version> - <sis.non-free.version>0.8</sis.non-free.version> + <sis.non-free.version>1.0-M1</sis.non-free.version> <geoapi.version>4.0-SNAPSHOT</geoapi.version> </properties> @@ -621,7 +632,7 @@ <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-surefire-plugin</artifactId> - <version>2.22.0</version> + <version>2.22.1</version> <configuration> <includes> <include>**/*TestSuite.java</include>