sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: `Variable.read(GridExtent area, int[] subsampling)` support reading character strings in addition of numerical value. This commit required refactoring for allowing more code sharing.
Date Tue, 20 Oct 2020 17:59:04 GMT
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 4587f99  `Variable.read(GridExtent area, int[] subsampling)` support reading character strings in addition of numerical value. This commit required refactoring for allowing more code sharing.
4587f99 is described below

commit 4587f99452e4e56592d0c64df63623372c57a3c5
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Oct 20 19:57:04 2020 +0200

    `Variable.read(GridExtent area, int[] subsampling)` support reading character strings in addition of numerical value.
    This commit required refactoring for allowing more code sharing.
---
 .../org/apache/sis/internal/netcdf/Convention.java |  29 ++-
 .../org/apache/sis/internal/netcdf/FeatureSet.java | 273 +++++++++++++--------
 .../org/apache/sis/internal/netcdf/Variable.java   |  51 +++-
 .../apache/sis/internal/netcdf/VariableRole.java   |   7 +-
 .../sis/internal/netcdf/impl/VariableInfo.java     | 135 +++++-----
 .../sis/internal/netcdf/ucar/VariableWrapper.java  |  49 +++-
 .../apache/sis/internal/netcdf/DataTypeTest.java   |  19 ++
 .../apache/sis/internal/netcdf/VariableTest.java   |  17 +-
 8 files changed, 397 insertions(+), 183 deletions(-)

diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java
index ebd410e..5dd4b56 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Convention.java
@@ -221,6 +221,9 @@ public class Convention {
      *       to confuse them with images.</li>
      * </ul>
      *
+     * <p>The default implementation returns {@link VariableRole#FEATURE} if the given variable may be values
+     * for one feature property of a feature set. This detection is based on the number of dimensions.</p>
+     *
      * @param  variable  the variable for which to get the role.
      * @return role of the given variable.
      */
@@ -228,16 +231,26 @@ public class Convention {
         if (variable.isCoordinateSystemAxis()) {
             return VariableRole.AXIS;
         }
-        int numVectors = 0;                                     // Number of dimension having more than 1 value.
-        for (final Dimension dimension : variable.getGridDimensions()) {
-            if (dimension.length() >= Grid.MIN_SPAN) {
-                numVectors++;
+        final int n = variable.getNumDimensions();
+        if (n == 1) {
+            if (variable.getAxisType() == null && variable.getAttributeValue(CF.AXIS) == null) {
+                return VariableRole.FEATURE;
             }
-        }
-        if (numVectors >= Grid.MIN_DIMENSION) {
+        } else if (n != 0) {
             final DataType dataType = variable.getDataType();
-            if (dataType.rasterDataType != null) {
-                return VariableRole.COVERAGE;
+            int numVectors = 0;                 // Number of dimension having more than 1 value.
+            for (final Dimension dimension : variable.getGridDimensions()) {
+                if (dimension.length() >= Grid.MIN_SPAN) {
+                    numVectors++;
+                }
+            }
+            if (numVectors >= Grid.MIN_DIMENSION) {
+                if (dataType.rasterDataType != null) {
+                    return VariableRole.COVERAGE;
+                }
+            }
+            if (n == 2 && dataType == DataType.CHAR) {
+                return VariableRole.FEATURE;
             }
         }
         return VariableRole.OTHER;
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java
index 5bc3a98..3e99730 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/FeatureSet.java
@@ -23,6 +23,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
+import java.util.HashMap;
 import java.util.Spliterator;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -52,6 +53,10 @@ import org.opengis.feature.FeatureType;
  * netCDF files encoded as specified in the OGC 16-114 (OGC Moving Features Encoding Extension: netCDF) specification.
  * This implementation is used as a fallback when the subclass does not provide a more specialized class.
  *
+ * <h4>Limitations</h4>
+ * Current implementation may perform many seek operations during traversal of feature instances.
+ * It may be inefficient unless the {@link Decoder} uses a {@code ChannelDataInput} backed by a direct buffer.
+ *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
  * @since   0.8
@@ -77,11 +82,11 @@ final class FeatureSet extends DiscreteSampling {
 
     /**
      * The singleton properties (for which there is only one value per feature instance), or an empty array if none.
-     * If non-empty, it typically contains a single variable which is the moving feature identifiers ("mfIdRef").
-     * If {@link #counts} is non-null, then the length of {@code identifiers[i]} variables shall be the same than
-     * the length of the {@link #counts} vector.
+     * In the case of trajectories, this array usually contains a single variable for the moving feature identifiers
+     * ("mfIdRef"). If {@link #counts} is non-null, then the length of {@code identifiers[i]} variables shall be the
+     * same than the length of the {@link #counts} vector.
      */
-    private final Variable[] identifiers;
+    private final Variable[] properties;
 
     /**
      * Whether the {@link #coordinates} array contains a temporal variable.
@@ -102,7 +107,7 @@ final class FeatureSet extends DiscreteSampling {
      * Any time-varying properties other than coordinate values, or an empty array if none.
      * All variables in this array shall have the same length than {@link #coordinates} variables.
      */
-    private final Variable[] properties;
+    private final Variable[] dynamicProperties;
 
     /**
      * The type of all features to be read by this {@code FeatureSet}.
@@ -117,24 +122,24 @@ final class FeatureSet extends DiscreteSampling {
      * is the name of the first dimension of {@code coordinates} and {@code properties} variables. All those
      * variables should have that first dimension in common, because {@code create(…)} uses that criterion.</p>
      *
-     * @param  decoder      the source of the features to create.
-     * @param  name         name to give to the feature type.
-     * @param  counts       the count of instances per feature, or {@code null} if none.
-     * @param  identifiers  the feature identifiers, possibly with other singleton properties.
-     * @param  coordinates  <var>x</var>, <var>y</var> and potentially <var>z</var> or <var>t</var> coordinate values.
-     * @param  hasTime      whether the {@code coordinates} array contains a temporal variable.
-     * @param  properties   the variables that contain custom time-varying properties.
+     * @param  decoder            the source of the features to create.
+     * @param  name               name to give to the feature type.
+     * @param  counts             the count of instances per feature, or {@code null} if none.
+     * @param  properties         variables providing a single value per feature instance (e.g. "mfIdRef").
+     * @param  coordinates        <var>x</var>, <var>y</var> and potentially <var>z</var> or <var>t</var> values.
+     * @param  hasTime            whether the {@code coordinates} array contains a temporal variable.
+     * @param  dynamicProperties  variables that contain time-varying properties other than coordinates.
      * @throws IllegalArgumentException if the given library is non-null but not available.
      */
-    private FeatureSet(final Decoder decoder, String name, final Vector counts, final Variable[] identifiers,
-                       final Variable[] coordinates, final boolean hasTime, final Variable[] properties)
+    private FeatureSet(final Decoder decoder, String name, final Vector counts, final Variable[] properties,
+                       final Variable[] coordinates, final boolean hasTime, final Variable[] dynamicProperties)
     {
         super(decoder.geomlib, decoder.listeners);
-        this.counts      = counts;
-        this.identifiers = identifiers;
-        this.coordinates = coordinates;
-        this.properties  = properties;
-        this.hasTime     = hasTime;
+        this.counts            = counts;
+        this.properties        = properties;
+        this.coordinates       = coordinates;
+        this.dynamicProperties = dynamicProperties;
+        this.hasTime           = hasTime;
         /*
          * Creates a description of the features to be read with following properties:
          *
@@ -142,8 +147,9 @@ final class FeatureSet extends DiscreteSampling {
          *    - Trajectory as a geometric object, potentially with a time characteristic.
          *    - Time-varying properties (i.e. properties having a value per instant).
          */
-        final FeatureTypeBuilder builder = new FeatureTypeBuilder(decoder.nameFactory, decoder.geomlib, decoder.listeners.getLocale());
-        for (final Variable v : identifiers) {
+        final FeatureTypeBuilder builder = new FeatureTypeBuilder(
+                decoder.nameFactory, decoder.geomlib, decoder.listeners.getLocale());
+        for (final Variable v : properties) {
             final Class<?> type = v.getDataType().getClass(v.getNumDimensions() > 1);
             describe(v, builder.addAttribute(type), false);
         }
@@ -155,12 +161,12 @@ final class FeatureSet extends DiscreteSampling {
                 geometry.addCharacteristic(MovingFeatures.TIME);
             }
         }
-        for (final Variable v : properties) {
+        for (final Variable v : dynamicProperties) {
             /*
              * Use `Number` type instead than a more specialized subclass because values
              * will be stored in `Vector` objects and that class implements `List<Number>`.
              */
-            final Class<?> type = v.isEnumeration() ? String.class : Number.class;
+            final Class<?> type = (v.isEnumeration() || v.isString()) ? String.class : Number.class;
             describe(v, builder.addAttribute(type).setMaximumOccurs(Integer.MAX_VALUE), hasTime);
         }
         /*
@@ -204,7 +210,11 @@ final class FeatureSet extends DiscreteSampling {
      */
     static FeatureSet[] create(final Decoder decoder) throws IOException, DataStoreException {
         final List<FeatureSet> features = new ArrayList<>(3);     // Will usually contain at most one element.
-search: for (final Variable counts : decoder.getVariables()) {
+        final Map<Dimension,Boolean> done = new HashMap<>();      // Whether a dimension has already been used.
+        for (final Variable v : decoder.getVariables()) {
+            if (v.getRole() != VariableRole.FEATURE) {
+                continue;
+            }
             /*
              * Any one-dimensional integer variable having a "sample_dimension" attribute string value
              * will be taken as an indication that we have Discrete Sampling Geometries. That variable
@@ -221,18 +231,36 @@ search: for (final Variable counts : decoder.getVariables()) {
              *         int counts(identifiers);
              *             counts:sample_dimension = "points";
              */
-            if (counts.getNumDimensions() == 1 && counts.getDataType().isInteger) {
-                final String sampleDimName = counts.getAttributeAsString(CF.SAMPLE_DIMENSION);
+            if (v.getNumDimensions() == 1 && v.getDataType().isInteger) {
+                final String sampleDimName = v.getAttributeAsString(CF.SAMPLE_DIMENSION);
                 if (sampleDimName != null) {
+                    // At this point, the variable is assumed to be `counts`.
+                    final Dimension featureDimension = v.getGridDimensions().get(0);
                     final Dimension sampleDimension = decoder.findDimension(sampleDimName);
                     if (sampleDimension != null) {
-                        addFeatureSet(features, decoder, counts, counts.getGridDimensions().get(0), sampleDimension);
+                        addFeatureSet(features, decoder, v, featureDimension, sampleDimension);
+                        done.put(sampleDimension, Boolean.TRUE);
                     } else {
                         decoder.listeners.warning(decoder.resources().getString(Resources.Keys.DimensionNotFound_3,
-                                                  decoder.getFilename(), counts.getName(), sampleDimName));
+                                                  decoder.getFilename(), v.getName(), sampleDimName));
                     }
+                    done.put(featureDimension, Boolean.TRUE);           // Overwrite `false` value with `true`.
+                    continue;
                 }
             }
+            done.putIfAbsent(v.getGridDimensions().get(0), Boolean.FALSE);
+        }
+        /*
+         * Above loop handled all features which seem to be trajectories (i.e. having a `counts` variable allowing
+         * each feature instance to contain an arbitrary number of points). If there is other feature variables not
+         * handled by above loop (i.e. feature properties without `counts` variable), the features are assumed to be
+         * "simple features" with only points instead than trajectories.
+         */
+        for (final Map.Entry<Dimension,Boolean> entry : done.entrySet()) {
+            if (!entry.getValue()) {
+                final Dimension dimension = entry.getKey();
+                addFeatureSet(features, decoder, null, dimension, dimension);
+            }
         }
         return features.toArray(new FeatureSet[features.size()]);
     }
@@ -263,15 +291,14 @@ search: for (final Variable counts : decoder.getVariables()) {
      * @param  counts            the count of instances per feature, or {@code null} if none.
      * @param  featureDimension  dimension of properties having a single value per feature instance.
      * @param  sampleDimension   dimension of properties having multiple values per feature instance.
-     * @return whether a {@code FeatureSet} has been added to the {@code features} collection.
      */
-    private static boolean addFeatureSet(final List<FeatureSet> features, final Decoder decoder, final Variable counts,
+    private static void addFeatureSet(final List<FeatureSet> features, final Decoder decoder, final Variable counts,
             final Dimension featureDimension, final Dimension sampleDimension) throws IOException, DataStoreException
     {
-        final String                 featureName = featureDimension.getName();
-        final boolean                isPointSet  = sampleDimension.equals(featureDimension);
-        final List<Variable>         singletons  = isPointSet ? Collections.emptyList() : new ArrayList<>();
-        final List<Variable>         properties  = new ArrayList<>();
+        final String         featureName = featureDimension.getName();
+        final boolean        isPointSet  = sampleDimension.equals(featureDimension);
+        final List<Variable> properties  = isPointSet ? Collections.emptyList() : new ArrayList<>();
+        final List<Variable> dynamicProperties   = new ArrayList<>();
         final Map<AxisType,Variable> coordinates = new EnumMap<>(AxisType.class);
         for (final Variable data : decoder.getVariables()) {
             if (data.equals(counts)) {
@@ -285,13 +312,13 @@ search: for (final Variable counts : decoder.getVariables()) {
              */
             if (featureName.equalsIgnoreCase(data.getName())) {
                 if (isScalarOrString(data, featureDimension, decoder)) {
-                    (isPointSet ? properties : singletons).add(data);
+                    (isPointSet ? dynamicProperties : properties).add(data);
                 }
             } else if (!isPointSet && isScalarOrString(data, featureDimension, null)) {
                 /*
                  * Feature property other than identifiers. Should rarely happen.
                  */
-                singletons.add(data);
+                properties.add(data);
             } else if (isScalarOrString(data, sampleDimension, null)) {
                 /*
                  * All other sample property (i.e. property having a value for each temporal value).
@@ -306,16 +333,15 @@ search: for (final Variable counts : decoder.getVariables()) {
                                                   decoder.getFilename(), axisType, previous.getName(), data.getName()));
                     }
                 } else {
-                    properties.add(data);
+                    dynamicProperties.add(data);
                 }
             }
         }
-        return features.add(new FeatureSet(decoder, featureName,
-                            (counts != null) ? counts.read() : null,
-                            toArray(singletons),
-                            toArray(coordinates.values()), coordinates.containsKey(AxisType.T),
-                            toArray(properties)));
-
+        features.add(new FeatureSet(decoder, featureName,
+                     (counts != null) ? counts.read() : null,
+                     toArray(properties),
+                     toArray(coordinates.values()), coordinates.containsKey(AxisType.T),
+                     toArray(dynamicProperties)));
     }
 
     /**
@@ -395,9 +421,9 @@ search: for (final Variable counts : decoder.getVariables()) {
         for (int i=0; ; i++) {
             final Variable[] data;
             switch (i) {
-                case 0: data = identifiers; break;
+                case 0: data = properties; break;
                 case 1: data = coordinates; break;
-                case 2: data = properties;  break;
+                case 2: data = dynamicProperties; break;
                 default: return OptionalLong.empty();
             }
             if (data.length != 0) {
@@ -426,115 +452,152 @@ search: for (final Variable counts : decoder.getVariables()) {
      */
     private final class Iter implements Spliterator<Feature> {
         /**
-         * Expected number of features.
+         * Expected number of feature instances.
          */
-        private final int count;
+        private final int size;
 
         /**
          * Index of the next feature to read.
          */
-        private int index;
+        private int nextIndex;
 
         /**
          * Position in the data vectors of the next feature to read.
          * This is the sum of the length of data in all previous features.
          */
-        private int position;
+        private long position;
 
         /**
-         * Values of all singleton properties (typically only identifiers), or an empty array if none.
+         * Values of all simple properties (having a single value per feature instance).
          *
-         * @see FeatureSet#identifiers
+         * @see FeatureSet#properties
+         */
+        private final List<?>[] propertyValues;
+
+        /**
+         * Names of feature properties where to store {@link #propertyValues}.
          */
-        private final List<?>[] idValues;
+        private final String[] propertyNames;
 
         /**
          * Creates a new iterator.
          */
         Iter() throws IOException, DataStoreException {
-            count = (int) Math.min(getFeatureCount().orElse(0), Integer.MAX_VALUE);
-            idValues = new List<?>[identifiers.length];
-            for (int i=0; i < idValues.length; i++) {
+            size = (int) Math.min(getFeatureCount().orElse(0), Integer.MAX_VALUE);
+            final int n = properties.length;
+            propertyValues = new List<?>[n];
+            propertyNames  = new String[n];
+            for (int i=0; i<n; i++) {
                 // Efficiency should be okay because those lists are cached.
-                idValues[i] = identifiers[i].readAnyType();
+                propertyValues[i] = properties[i].readAnyType();
+                propertyNames [i] = properties[i].getName();
             }
         }
 
         /**
          * Executes the given action only on the next feature, if any.
          *
+         * <h4>Limitations</h4>
+         * Current implementation performs a lot of seek operations, which may be inefficient
+         * unless the {@link Decoder} uses a {@code ChannelDataInput} backed by a direct buffer.
+         *
          * @throws ArithmeticException if the size of a variable exceeds {@link Integer#MAX_VALUE}, or other overflow occurs.
          * @throws BackingStoreException if an {@link IOException} or {@link DataStoreException} occurred.
-         *
-         * @todo current reading process implies lot of seeks, which is inefficient.
          */
         @Override
         public boolean tryAdvance(final Consumer<? super Feature> action) {
-            final Vector[] coordinateValues  = new Vector[coordinates.length];
-            final Object[] singleProperties  = new Object[identifiers.length];
-            final Object[] varyingProperties = new Object[properties .length];
-            for (int i=0; i < singleProperties.length; i++) {
-                singleProperties[i] = idValues[i].get(index);
-            }
-            final int[] step = {1};
-            boolean isEmpty = true;
-            int length;
+            final Feature feature = type.newInstance();
+            boolean isEmpty;
+            long length;
             do {
-                length = (counts != null) ? counts.intValue(index) : 1;
-                if (length != 0) {
-                    isEmpty = false;
-                    final GridExtent extent = new GridExtent(null, new long[] {position},
-                                    new long[] {Math.addExact(position, length)}, false);
-                    try {
-                        for (int i=0; i<coordinateValues.length; i++) {
-                            coordinateValues[i] = coordinates[i].read(extent, step);
-                        }
-                        for (int i=0; i<properties.length; i++) {
-                            final Variable p = properties[i];
-                            final Vector data = p.read(extent, step);
-                            if (p.isEnumeration()) {
-                                final String[] meanings = new String[data.size()];
-                                for (int j=0; j<meanings.length; j++) {
-                                    String m = p.meaning(data.intValue(j));
-                                    meanings[j] = (m != null) ? m : "";
-                                }
-                                varyingProperties[i] = Arrays.asList(meanings);
-                            } else {
-                                varyingProperties[i] = data;
-                            }
-                        }
-                    } catch (IOException | DataStoreException e) {
-                        throw new BackingStoreException(canNotReadFile(), e);
-                    }
-                }
-                if (++index >= count) {
+                if (nextIndex >= size) {
                     return false;
                 }
+                length  = (counts != null) ? counts.longValue(nextIndex) : 1;
+                isEmpty = (length == 0);
+                for (int i=0; i < propertyNames.length; i++) {
+                    final Object value = propertyValues[i].get(nextIndex);
+                    feature.setPropertyValue(propertyNames[i], value);
+                    if (isEmpty) {
+                        isEmpty = (value == null) || "".equals(value) ||
+                                  (value instanceof Float  && ((Float)  value).isNaN()) ||
+                                  (value instanceof Double && ((Double) value).isNaN());
+                    }
+                }
+                nextIndex++;
             } while (isEmpty);
             /*
              * At this point we found that there is some data we can put in a feature instance.
+             * Above loop has set the static properties (those having one value per feature).
+             * Now set the dynamic properties (those having time-varying values).
              */
-            final Feature feature = type.newInstance();
-            for (int i=0; i < singleProperties.length; i++) {
-                feature.setPropertyValue(identifiers[i].getName(), singleProperties[i]);
-            }
-            for (int i=0; i<properties.length; i++) {
-                feature.setPropertyValue(properties[i].getName(), varyingProperties[i]);
-                // TODO: set time characteristic.
+            final Vector[] coordinateValues = new Vector[coordinates.length];
+            final GridExtent extent = extent(null, 1, length);
+            List<Dimension> textDimensions = null;
+            GridExtent textExtent = null;
+            try {
+                for (int i=0; i<coordinateValues.length; i++) {
+                    coordinateValues[i] = coordinates[i].read(extent, null);
+                }
+                for (int i=0; i<dynamicProperties.length; i++) {
+                    final Variable p = dynamicProperties[i];
+                    Object value;
+                    if (p.getNumDimensions() > 1) {
+                        final List<Dimension> dimensions = p.getGridDimensions();
+                        if (textExtent == null || !dimensions.equals(textDimensions)) {
+                            textExtent = extent(dimensions, dimensions.size(), length);
+                            textDimensions = dimensions;
+                        }
+                        value = p.readAnyType(textExtent, null);
+                    } else {
+                        value = p.readAnyType(extent, null);
+                    }
+                    if (p.isEnumeration() && value instanceof Vector) {
+                        final Vector data = (Vector) value;
+                        final String[] meanings = new String[data.size()];
+                        for (int j=0; j<meanings.length; j++) {
+                            String m = p.meaning(data.intValue(j));
+                            meanings[j] = (m != null) ? m : "";
+                        }
+                        value = Arrays.asList(meanings);
+                    }
+                    feature.setPropertyValue(dynamicProperties[i].getName(), value);
+                    // TODO: set time characteristic.
+                }
+            } catch (IOException | DataStoreException e) {
+                throw new BackingStoreException(canNotReadFile(), e);
             }
             // TODO: temporary hack - to be replaced by support in Vector.
             final int dimension = coordinates.length - (hasTime ? 1 : 0);
-            final double[] tmp = new double[length * dimension];
+            final double[] tmp = new double[Math.toIntExact(length * dimension)];
             for (int i=0; i<tmp.length; i++) {
                 tmp[i] = coordinateValues[i % dimension].doubleValue(i / dimension);
             }
             feature.setPropertyValue(TRAJECTORY, factory.createPolyline(false, dimension, Vector.create(tmp)));
             action.accept(feature);
-            position = Math.addExact(position, length);
+            position += length;         // Check for ArithmeticException is already done by `extent(…)` call.
             return true;
         }
 
         /**
+         * Creates a grid extent for a sub-region to read in a vector of the given number of dimensions.
+         *
+         * @param  dimensions  dimensions of the vector to read. Can be {@code null} if {@code n} is 1.
+         * @param  n           number of dimensions in the vector to read.
+         * @param  length      number of values to read.
+         */
+        private GridExtent extent(final List<Dimension> dimensions, int n, final long length) {
+            final long[] lower = new long[n];
+            final long[] upper = new long[n];
+            lower[--n] = position;
+            upper[  n] = Math.addExact(position, length);
+            for (int i=0; i<n; i++) {
+                upper[i] = dimensions.get(n-i).length();
+            }
+            return new GridExtent(null, lower, upper, false);
+        }
+
+        /**
          * Current implementation can not split this iterator.
          */
         @Override
@@ -547,7 +610,7 @@ search: for (final Variable counts : decoder.getVariables()) {
          */
         @Override
         public long estimateSize() {
-            return count - index;
+            return size - nextIndex;
         }
 
         /**
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
index d964ee4..6576f5d 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
@@ -436,6 +436,18 @@ public abstract class Variable extends Node {
     }
 
     /**
+     * Returns {@code true} if this variable should be considered as a list of strings.
+     *
+     * <div class="note"><b>Maintenance note:</b>
+     * the implementation of this method is inlined in some places, when the code already
+     * has the {@link DataType} value at hand. If this implementation is modified, search
+     * for {@link DataType#CHAR} usage.</div>
+     */
+    final boolean isString() {
+        return getDataType() == DataType.CHAR && getNumDimensions() >= 2;
+    }
+
+    /**
      * Returns {@code true} if this variable is an enumeration.
      *
      * @return whether this variable is an enumeration.
@@ -972,8 +984,8 @@ public abstract class Variable extends Node {
      * method shall {@linkplain #replaceNaN(Object) replace fill/missing values by NaN values}.
      *
      * @param  area         indices of cell values to read along each dimension, in "natural" order.
-     * @param  subsampling  subsampling along each dimension. 1 means no subsampling.
-     * @return the data as an array of a Java primitive type.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as a vector wrapping a Java array.
      * @throws IOException if an error occurred while reading the data.
      * @throws DataStoreException if a logical error occurred.
      * @throws ArithmeticException if the size of the region to read exceeds {@link Integer#MAX_VALUE}, or other overflow occurs.
@@ -981,6 +993,19 @@ public abstract class Variable extends Node {
     public abstract Vector read(GridExtent area, int[] subsampling) throws IOException, DataStoreException;
 
     /**
+     * Reads a subsampled sub-area of the variable and returns them as a list of any object.
+     * Elements in the returned list may be {@link Number} or {@link String} instances.
+     *
+     * @param  area         indices of cell values to read along each dimension, in "natural" order.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as a list of {@link Number} or {@link String} instances.
+     * @throws IOException if an error occurred while reading the data.
+     * @throws DataStoreException if a logical error occurred.
+     * @throws ArithmeticException if the size of the region to read exceeds {@link Integer#MAX_VALUE}, or other overflow occurs.
+     */
+    public abstract List<?> readAnyType(GridExtent area, int[] subsampling) throws IOException, DataStoreException;
+
+    /**
      * Reads all the data for this variable and returns them as an array of a Java primitive type.
      * This is the implementation of {@link #read()} method, invoked when the value is not cached.
      *
@@ -1004,11 +1029,11 @@ public abstract class Variable extends Node {
             if (n >= 2) {
                 final List<Dimension> dimensions = getGridDimensions();
                 final int length = Math.toIntExact(dimensions.get(--n).length());
-                int count = Math.toIntExact(dimensions.get(--n).length());
+                long count = dimensions.get(--n).length();
                 while (n > 0) {
-                    count = Math.multiplyExact(count, Math.toIntExact(dimensions.get(--n).length()));
+                    count = Math.multiplyExact(count, dimensions.get(--n).length());
                 }
-                final String[] strings = createStringArray(array, count, length);
+                final String[] strings = createStringArray(array, Math.toIntExact(count), length);
                 /*
                  * Following method calls take the array reference without cloning it.
                  * Consequently creating those two objects now (even if we may not use them) is reasonably cheap.
@@ -1068,6 +1093,22 @@ public abstract class Variable extends Node {
     protected abstract String[] createStringArray(Object chars, int count, int length);
 
     /**
+     * Creates a list of character strings from a "two-dimensional" array of characters stored in a flat array.
+     *
+     * @param  chars  the "two-dimensional" array stored in a flat {@code byte[]} or {@code char[]} array.
+     * @param  area   the {@code area} argument given to the {@code read(…)} method that obtained the array.
+     * @return list of character strings.
+     */
+    protected final List<String> createStringList(final Object chars, final GridExtent area) {
+        final int length = Math.toIntExact(area.getSize(0));
+        long count = area.getSize(1);
+        for (int i = area.getDimension(); --i >= 2;) {          // As a safety, but should never enter in this loop.
+            count = Math.multiplyExact(count, area.getSize(i));
+        }
+        return UnmodifiableArrayList.wrap(createStringArray(chars, Math.toIntExact(count), length));
+    }
+
+    /**
      * Wraps the given data in a {@link Vector} with the assumption that accuracy in base 10 matters.
      * This method is suitable for coordinate axis variables, but should not be used for the main data.
      *
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java
index 380d30a..675575b 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/VariableRole.java
@@ -21,7 +21,7 @@ package org.apache.sis.internal.netcdf;
  * Specifies whether a variable is used as a coordinate system axis, a coverage or other purpose.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
  * @since   1.0
  * @module
  */
@@ -37,6 +37,11 @@ public enum VariableRole {
     COVERAGE,
 
     /**
+     * The variable is a property of a feature.
+     */
+    FEATURE,
+
+    /**
      * Unidentified kind of variable.
      */
     OTHER
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
index ee2577e..1426588 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
@@ -429,7 +429,7 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo> {
     @Override
     protected String getAxisType() {
         final Object value = getAttributeValue(_Coordinate.AxisType, "_coordinateaxistype");
-        return (value instanceof String) ? (String) value : null;
+        return (value != null) ? value.toString() : null;
     }
 
     /**
@@ -571,58 +571,14 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo> {
      * Reads all the data for this variable and returns them as an array of a Java primitive type.
      * Multi-dimensional variables are flattened as a one-dimensional array (wrapped in a vector).
      * Fill values/missing values are replaced by NaN if {@link #hasRealValues()} is {@code true}.
-     * The vector is cached and returned as-is in all future invocation of this method.
      *
      * @throws ArithmeticException if the size of the variable exceeds {@link Integer#MAX_VALUE}, or other overflow occurs.
      *
      * @see #read()
      */
     @Override
-    protected Object readFully() throws IOException, DataStoreContentException {
-        if (reader == null) {
-            throw new DataStoreContentException(unknownType());
-        }
-        final int    dimension   = dimensions.length;
-        final long[] lower       = new long[dimension];
-        final long[] upper       = new long[dimension];
-        final int [] subsampling = new int [dimension];
-        for (int i=0; i<dimension; i++) {
-            upper[i] = dimensions[(dimension - 1) - i].length();
-            subsampling[i] = 1;
-        }
-        final Region region = new Region(upper, lower, upper, subsampling);
-        applyUnlimitedDimensionStride(region);
-        Object array = reader.read(region);
-        replaceNaN(array);
-        /*
-         * If we can convert a double[] array to a float[] array, we should do that before
-         * to invoke `setValues(array)` - we can not rely on data.compress(tolerance). The
-         * reason is because we assume that float[] arrays are accurate in base 10 even if
-         * the data were originally stored as doubles. The Vector class does not make such
-         * assumption since it is specific to what we observe with netCDF files. To enable
-         * this assumption, we need to convert to float[] before createDecimalVector(…).
-         */
-        if (array instanceof double[]) {
-            final float[] copy = ArraysExt.copyAsFloatsIfLossless((double[]) array);
-            if (copy != null) array = copy;
-        }
-        return array;
-    }
-
-    /**
-     * If this variable uses the unlimited dimension, we have to skip the records of all other unlimited variables
-     * before to reach the next record of this variable.  Current implementation can do that only if the number of
-     * bytes to skip is a multiple of the data type size. It should be the case most of the time because variables
-     * in netCDF files have a 4 bytes padding. It may not work however if the variable uses {@code long} or
-     * {@code double} type.
-     */
-    private void applyUnlimitedDimensionStride(final Region region) throws DataStoreContentException {
-        if (isUnlimited()) {
-            if (offsetToNextRecord < 0) {
-                throw canNotComputePosition(null);
-            }
-            region.setAdditionalByteOffset(dimensions.length - 1, offsetToNextRecord);
-        }
+    protected Object readFully() throws IOException, DataStoreException {
+        return readArray(null, null);
     }
 
     /**
@@ -637,9 +593,48 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo> {
      */
     @Override
     public Vector read(final GridExtent area, final int[] subsampling) throws IOException, DataStoreException {
+        return Vector.create(readArray(area, subsampling), dataType.isUnsigned);
+    }
+
+    /**
+     * Reads a subsampled sub-area of the variable and returns them as a list of any object.
+     * Elements in the returned list may be {@link Number} or {@link String} instances.
+     *
+     * @param  area         indices of cell values to read along each dimension, in "natural" order.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as a list of {@link Number} or {@link String} instances.
+     */
+    @Override
+    public List<?> readAnyType(final GridExtent area, final int[] subsampling) throws IOException, DataStoreException {
+        final Object array = readArray(area, subsampling);
+        if (dataType == DataType.CHAR && dimensions.length >= 2) {
+            return createStringList(array, area);
+        }
+        return Vector.create(array, dataType.isUnsigned);
+    }
+
+    /**
+     * Reads the data from this variable and returns them as an array of a Java primitive type.
+     * Multi-dimensional variables are flattened as a one-dimensional array (wrapped in a vector).
+     * Fill values/missing values are replaced by NaN if {@link #hasRealValues()} is {@code true}.
+     * Array elements are in "natural" order (inverse of netCDF order).
+     *
+     * @param  area         indices (in "natural" order) of cell values to read, or {@code null} for whole variable.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none. Ignored if {@code area} is null.
+     * @return the data as an array of a Java primitive type.
+     * @throws ArithmeticException if the size of the variable exceeds {@link Integer#MAX_VALUE}, or other overflow occurs.
+     *
+     * @see #read()
+     * @see #read(GridExtent, int[])
+     */
+    private Object readArray(final GridExtent area, int[] subsampling) throws IOException, DataStoreException {
         if (reader == null) {
             throw new DataStoreContentException(unknownType());
         }
+        final int dimension = dimensions.length;
+        final long[] lower  = new long[dimension];
+        final long[] upper  = new long[dimension];
+        final long[] size   = (area != null) ? new long[dimension] : upper;
         /*
          * NetCDF sorts datas in reverse dimension order. Example:
          *
@@ -659,20 +654,46 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo> {
          *   (2,0,0) (2,0,1) (2,0,2) (2,0,3)
          *   (2,1,0) (2,1,1) (2,1,2) (2,1,3)
          */
-        final int dimension = dimensions.length;
-        final long[] size  = new long[dimension];
-        final long[] lower = new long[dimension];
-        final long[] upper = new long[dimension];
         for (int i=0; i<dimension; i++) {
-            lower[i] = area.getLow(i);
-            upper[i] = Math.incrementExact(area.getHigh(i));
-            size [i] = dimensions[(dimension - 1) - i].length();
+            size[i] = dimensions[(dimension - 1) - i].length();
+            if (area != null) {
+                lower[i] = area.getLow(i);
+                upper[i] = Math.incrementExact(area.getHigh(i));
+            }
+        }
+        if (subsampling == null) {
+            subsampling = new int[dimension];
+            Arrays.fill(subsampling, 1);
         }
         final Region region = new Region(size, lower, upper, subsampling);
-        applyUnlimitedDimensionStride(region);
-        final Object array = reader.read(region);
+        /*
+         * If this variable uses the unlimited dimension, we have to skip the records of all other unlimited variables
+         * before to reach the next record of this variable.  Current implementation can do that only if the number of
+         * bytes to skip is a multiple of the data type size. It should be the case most of the time because variables
+         * in netCDF files have a 4 bytes padding. It may not work however if the variable uses {@code long} or
+         * {@code double} type.
+         */
+        if (isUnlimited()) {
+            if (offsetToNextRecord < 0) {
+                throw canNotComputePosition(null);
+            }
+            region.setAdditionalByteOffset(dimensions.length - 1, offsetToNextRecord);
+        }
+        Object array = reader.read(region);
         replaceNaN(array);
-        return Vector.create(array, dataType.isUnsigned);
+        if (area == null && array instanceof double[]) {
+            /*
+             * If we can convert a double[] array to a float[] array, we should do that before
+             * to invoke `setValues(array)` - we can not rely on data.compress(tolerance). The
+             * reason is because we assume that float[] arrays are accurate in base 10 even if
+             * the data were originally stored as doubles. The Vector class does not make such
+             * assumption since it is specific to what we observe with netCDF files. To enable
+             * this assumption, we need to convert to float[] before createDecimalVector(…).
+             */
+            final float[] copy = ArraysExt.copyAsFloatsIfLossless((double[]) array);
+            if (copy != null) array = copy;
+        }
+        return array;
     }
 
     /**
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
index 37b0ec5..c9c533a 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/VariableWrapper.java
@@ -488,28 +488,65 @@ final class VariableWrapper extends Variable {
      * Array elements are in inverse of netCDF order.
      *
      * @param  area         indices of cell values to read along each dimension, in "natural" order.
-     * @param  subsampling  subsampling along each dimension. 1 means no subsampling.
-     * @return the data as an array of a Java primitive type.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as a vector wrapping a Java array.
      */
     @Override
     public Vector read(final GridExtent area, final int[] subsampling) throws IOException, DataStoreException {
+        final Object array = readArray(area, subsampling);
+        return Vector.create(array, variable.isUnsigned());
+    }
+
+    /**
+     * Reads a subsampled sub-area of the variable and returns them as a list of any object.
+     * Elements in the returned list may be {@link Number} or {@link String} instances.
+     *
+     * @param  area         indices of cell values to read along each dimension, in "natural" order.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as a list of {@link Number} or {@link String} instances.
+     */
+    @Override
+    public List<?> readAnyType(final GridExtent area, final int[] subsampling) throws IOException, DataStoreException {
+        final Object array = readArray(area, subsampling);
+        if (variable.getDataType() == ucar.ma2.DataType.CHAR && variable.getRank() >= 2) {
+            return createStringList(array, area);
+        }
+        return Vector.create(array, variable.isUnsigned());
+    }
+
+    /**
+     * Reads the data from this variable and returns them as an array of a Java primitive type.
+     * Multi-dimensional variables are flattened as a one-dimensional array (wrapped in a vector).
+     * This method may replace fill/missing values by NaN values and caches the returned vector.
+     *
+     * @param  area         indices of cell values to read along each dimension, in "natural" order.
+     * @param  subsampling  subsampling along each dimension, or {@code null} if none.
+     * @return the data as an array of a Java primitive type.
+     *
+     * @see #read()
+     * @see #read(GridExtent, int[])
+     */
+    private Object readArray(final GridExtent area, final int[] subsampling) throws IOException, DataStoreException {
         int n = area.getDimension();
         final int[] lower = new int[n];
         final int[] size  = new int[n];
-        final int[] sub   = new int[n--];
+        final int[] sub   = (subsampling != null) ? new int[n] : null;
+        n--;
         for (int i=0; i<=n; i++) {
             final int j = (n - i);
             lower[j] = Math.toIntExact(area.getLow(i));
             size [j] = Math.toIntExact(area.getSize(i));
-            sub  [j] = subsampling[i];
+            if (sub != null) {
+                sub[j] = subsampling[i];
+            }
         }
         final Array array;
         try {
-            array = variable.read(new Section(lower, size, sub));
+            array = variable.read(sub != null ? new Section(lower, size, sub) : new Section(lower, size));
         } catch (InvalidRangeException e) {
             throw new DataStoreException(e);
         }
-        return Vector.create(get1DJavaArray(array), variable.isUnsigned());
+        return get1DJavaArray(array);
     }
 
     /**
diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/DataTypeTest.java b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/DataTypeTest.java
index 6ce401d..98b2522 100644
--- a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/DataTypeTest.java
+++ b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/DataTypeTest.java
@@ -85,6 +85,25 @@ public final strictfp class DataTypeTest extends TestCase {
     }
 
     /**
+     * Tests {@link DataType#isInteger}.
+     */
+    @Test
+    public void testIsNumber() {
+        assertTrue (DataType.BYTE  .isInteger);
+        assertTrue (DataType.UBYTE .isInteger);
+        assertTrue (DataType.SHORT .isInteger);
+        assertTrue (DataType.USHORT.isInteger);
+        assertTrue (DataType.INT   .isInteger);
+        assertTrue (DataType.UINT  .isInteger);
+        assertTrue (DataType.INT64 .isInteger);
+        assertTrue (DataType.UINT64.isInteger);
+        assertFalse(DataType.FLOAT .isInteger);
+        assertFalse(DataType.DOUBLE.isInteger);
+        assertFalse(DataType.CHAR  .isInteger);
+        assertFalse(DataType.STRING.isInteger);
+    }
+
+    /**
      * Verifies the {@link DataType#classe} values.
      */
     @Test
diff --git a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/VariableTest.java b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/VariableTest.java
index d13313a..3b13e45 100644
--- a/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/VariableTest.java
+++ b/storage/sis-netcdf/src/test/java/org/apache/sis/internal/netcdf/VariableTest.java
@@ -39,7 +39,7 @@ import static org.opengis.test.Assert.*;
  * {@link #createDecoder(TestData)} method in order to test a different implementation.
  *
  * @author  Martin Desruisseaux (Geomatys)
- * @version 1.0
+ * @version 1.1
  * @since   0.3
  * @module
  */
@@ -310,6 +310,7 @@ public strictfp class VariableTest extends TestCase {
         final Variable variable = selectDataset(TestData.NETCDF_2D_GEOGRAPHIC).getVariables()[2];
         assertEquals("lon", variable.getName());
         final Vector data = variable.read();
+        assertSame(data, variable.readAnyType());
         assertEquals("lon", Float.class, data.getElementType());
         final int length = data.size();
         assertEquals("length", 73, length);
@@ -317,4 +318,18 @@ public strictfp class VariableTest extends TestCase {
             assertEquals("Longitude value", -180 + 5*i, data.floatValue(i), 0f);
         }
     }
+
+    /**
+     * Tests {@link Variable#readAnyType()} on strings.
+     *
+     * @throws IOException if an error occurred while reading the netCDF file.
+     * @throws DataStoreException if a logical error occurred.
+     */
+    @Test
+    public void testReadStrings() throws IOException, DataStoreException {
+        final Variable variable = selectDataset(TestData.MOVING_FEATURES).findVariable("features");
+        assertEquals("features", variable.getName());
+        final List<?> identifiers = variable.readAnyType();
+        assertArrayEquals(new String[] {"a4078a16", "1e146c16", "f50ff004", "", ""}, identifiers.toArray());
+    }
 }


Mime
View raw message