On 11/11/06, Martin Desruisseaux <[EMAIL PROTECTED]> wrote:
> I'm very late, but I finally started to review the coverage branch merge. I
> would like to hold on the 2.3 release until I finished this review, since I
> have
> some questions.
>
> org.geotools.coverage.CategoryList
> ----------------------------------
> The implementation before merge was looking for a "no data" category using
> Double.isNaN(geophysicsValue) as a criterion ("geophysics values" not to be
> confused with "pixel values", which can't be NaN). The coverage branch merge
> added a check based on the "no data" name in current local, regardless of
> Double.isNaN(...). Should we move this check into the "if (Double.isNaN(...))"
> block? By definition, a "no data" category maps to NaN geophysics values. If
> it
> maps to real values, then it is not a "no data" category anymore.
>
> Is there any use case for selecting a "no data" category which maps to real
> geophysics values? Or can I move the check into the Double.isNaN(...) block?
>
Well, in this is the sentence "by definition" may have a different
meaning for different people because it is at the same time subjective
as well as dependent on the data source. Some examples, in GTOPO30 the
nodata value is -9999 and the same applies to esri ascii grid files,
hence, in general it is important to be able to handle values
indicating the no data value that are not real "NoData" i.e. NaN. Many
scientists I have met really want to use value different from NaN for
indicating the missing value (hey, do not ask me why, it is their will
:-) ) hence I only tried to reflect what happens in reality.
To summarise, I would remove this check.
>
> org.geotools.coverage.grid.GeneralGridGeometry
> ----------------------------------------------
> The coverage branch merge turned a number of "package private static" methods
> into public ones:
>
> - reverse
> - swapXY
>
> Do those methods really need to be public? They were for internal purpose, and
> it is not clear to me what a user way want to do with 'reverse' for example.
>
Well, I see that you never fought with the reversed-axis problem but
believe me when everybody understands lon,lat while our crs where
mostly lat,lon these methods come pretty hand to reverse things .-).
As of now I am starting to remove the from most plugins but they are
still useful.
There are formats like GeoTiff where crs order is ALWAYS lon,lat as
well as for ascii grids and gtopo30. These methods are useful for
understanding if and how to manage crs reversion inside plugins. I
agree they are not for normal users but I believe that everybody
sooner or later fofund himself in the situation where he needed some
cool complex methods from a library and this methods were not
accessible. I think different users (and developers) want to have
different level of access to the API so why hiding a lof method which
are utility methods?
> Additionnaly, I noticed that the coverage branch merge added new public static
> method, with code that were previously done by constructors:
>
> - getEnvelope(GridRange, MathTransform, CoordinateReferenceSystem, boolean)
> - getTransform(GridRange, Envelope, boolean[], boolean, boolean)
>
> The signature are identical to the constructor signature except for the
> additional boolean argument. Actually, those public static method are just
> shorthand for the following existing methods:
>
> - new GeneralGridGeometry(gridRange, mathTransform, crs).getEnvelope();
> - new GeneralGridGeometry(gridRange, envelope, reverse,
> swapXY).getTransform();
>
> Their only advantage is to avoid the temporary creation of a
> GeneralGridGeometry
> object, at the cost of more complex API (at least duplication in methods
> signature) and less flexibility (are we going to add a 'getEnvelope(...)' or
> 'getTransform(...)' method every time we add a new constructor signature?)
>
> Unless there is a real performance bootleneck, can we ask peoples to use the
> new
> GridGeometry(...).getFoo() idiom, which is more extensible? The only thing we
> would need to add is a
>
> getTransform(PixelOrientation)
>
> method in order to provides the functionality provided by the new 'boolean
> halfPix' argument, where PixelOrientation is:
>
> http://geoapi.sourceforge.net/snapshot/javadoc/org/opengis/metadata/spatial/PixelOrientation.html
>
Well you went right to the point of this thing, performances. I use
this methods to produce envelopes from GridRange and MathTransform and
to produce Transforms from Envelopes ans GridRange in in many places.
I am writing from memory but I think we are using it inside most most
coverage plugins to build envelopes from world files, inside the
StreamingRenderer. Consider also that GeoServer sits on top of all
these components, hence if we can avoid creating useless objects we
should do that because in a server side environment creating useless
object is something we want to avoid. You know what, we could move
this methods in a utility class much like the CRS or CRSUtilies in
order to have clear in mind that these methods are just utilities
methods.
> Martin
>
--
-------------------------------------------------------
Eng. Simone Giannecchini
President /CEO GeoSolutions
http://www.geo-solutions.it
-------------------------------------------------------
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
