HdfEos handling of featureType attribute

[rschmunk]

I have a couple sample HDF-EOS files containing what is essentially featureType trajectory data. However, netCDF-Java is adding a :featureType = "PROFILE" global attribute to the files. Poking around in the code, I find that this comes from the HdfEos class examining the geolocation info, finding that the lat and lon axes are not 2D and defaulting to PROFILE:

    if ((latAxis != null) && (lonAxis != null)) {
        int xyDomainSize = CoordinateSystem.countDomain(new Variable[]{latAxis, lonAxis});
        if (xyDomainSize < 2) featureType = FeatureType.PROFILE;  // ??
    }

Note the double question mark in the comment. It would seem that when this was written, someone wasn't entirely sure that they were doing the right thing?

I'm looking at fixing this by having the geolocation examination also look for a time axes, and then replacing the above with

        if ((latAxis != null) && (lonAxis != null)) {
            log.trace ("found lonAxis and latAxis");
            List<Dimension> xyDomain = CoordinateSystem.makeDomain(new Variable[] {latAxis, lonAxis});
            log.trace ("xyDomain {}", xyDomain);
            if (xyDomain.size() < 2) {
              if (timeAxis != null)
              {
                Dimension dd1 = timeAxis.getDimension(0);
                Dimension dd2 = latAxis.getDimension(0);
                Dimension dd3 = lonAxis.getDimension(0);

                if (dd1.equals (dd2) && dd1.equals(dd3)) {
                    featureType = FeatureType.TRAJECTORY;  // ??
                } else {
                    featureType = FeatureType.PROFILE;  // ??
                }
              } else {
                featureType = FeatureType.PROFILE;  // ??
              }
            }
        }

This solution isn't perfect — certainly it omits the possibility of other featureTypes — but it should be an improvement over what's in HdfEos now.

Any comments, thoughts, criticism?

[lesserwhirls]

I think you've interpreted the question marks correctly - it's much worse to see:

// LOOK - WTF?!?

which generally means the author didn't know why or could not remember why that line was needed or what it was trying to do.

I think your code looks reasonable to me (I'd change the log.trace calls to debug before pulling it in), and if it works for your case, then I think the next step would be for us to run it against our test full test suite to make sure nothing in there is impacted. Do you have this change in a repo on github? If so, I can go ahead and get the test suite running (it takes roughly two hours to run).

Speaking of tests...would you happen to have some small files that exercise this?

Thanks @msdsoftware!

[rschmunk]

@lesserwhirls, The change is only implemented in my local copy of thredds-master. I'll make the changes to my fork on Github later today.

Will include some comments about other featureTypes being not yet implemented, e.g., also check for a vertical axis so that one can be more sure it's really a profile, or perhaps a trajectoryProfile.

[rschmunk]

@lesserwhirls, I submitted a pull request, but if you want to run tests first then you can go to my fork.

A sample file (280kB) which ought to labeled with featureType="TRAJECTORY" is at https://www.dropbox.com/s/3tf70xd1kso2y1c/OMSO2_2010m0116t1909-o29293_v003-2014m0924t104451vector.he5?dl=0