Opened 12 years ago

Closed 11 years ago

#1653 closed enhancement (fixed)

[raster] ST_Transform & ST_Resample are confusing

Reported by: pracine Owned by: pracine
Priority: high Milestone: PostGIS 2.1.0
Component: raster Version: master
Keywords: history Cc:

Description

Users expect to use ST_Transform to reproject to a new srid and they want to be able to define the alignment of the new grid at the same time. However this is now possible only with ST_Resample.

ST_Transform should accept more alignment parameters and ST_Resample should not have a srid parameter.

I understand that those two functions are very much aligned on what GDAL provides but gdalwarp is itself very confusing. We don't want to reproduce this confusion.

Attachments (1)

sb.patch (2.4 KB ) - added by pramsey 12 years ago.
Attempt at using special windows functions for Win64 case…

Download all attachments as: .zip

Change History (13)

comment:1 by pracine, 12 years ago

Priority: mediumcritical

I want comment from Bborie, Paul & Regina before doing something here. Shall we change ST_Resample & ST_Transform signatures or not?

comment:2 by robe, 12 years ago

I thought we weren't going to be changing signatures in beta phase? Paul your comment? If we do I demand we have an RC phase where absolutely no signature changes are allowed.

comment:3 by pracine, 12 years ago

Ok I reformulate my question: "In case we could still change some function signatures, shall we change ST_Resample & ST_Transform signatures or not?"

comment:4 by pracine, 12 years ago

I must also say that it might be better to change them during the beta period at the cost of braking one rule than being stock with them forever.

by pramsey, 12 years ago

Attachment: sb.patch added

Attempt at using special windows functions for Win64 case…

comment:5 by pramsey, 12 years ago

Please try that patch, then we should decide. I've tracked down the "problem" with the portable snprintf, which is that it only supports exponents between -99 and 99. Which is more than enough size for our purposes, but we have a KML test that exercises 1e300, which I've now determined is only just shy of the maximum that OSX will handle, 1e306. Basically, it's a semi-bogus test. I have a sandbox ready to bring the portable snprintf solution in, which will handle more cases than just the Win64 case. My hack sb.patch above is very specific to the bug and platform as reported by Regina.

comment:6 by Bborie Park, 12 years ago

pramsey, I think you're posting this to the wrong ticket… maybe #1668?

comment:7 by robe, 12 years ago

Pierre,

I'm personally fine with living with these signatures forever. There are a whole bunch of worse signatures we are living with that irritate me much more than this (like the assumption of band 1 if none specified in some functions and assumption of all in others). My main concern is people have started using raster in production, and this may screw them (screw me in fact since now I've got to check my code to make sure I'm not using them). It's one thing to add things, it's much worse to take them away during beta phase.

However if you feel strongly about it, you should take them out of the documentation and then perhaps in 2.1 or 3.0 we can remove them.

comment:8 by pracine, 12 years ago

Milestone: PostGIS 2.0.0PostGIS 2.1.0

Then I postpone it to 2.1…

comment:9 by Bborie Park, 11 years ago

Priority: criticalhigh

ST_Resample is the key underlying function exposing the GDAL Warp API. ST_Transform runs on top of ST_Resample and provides a subset of ST_Resample's capabilties. ST_Resample is the full toolbox so has lots of options, thus causing confusion. I don't believe there is any way around this confusion.

comment:10 by pracine, 11 years ago

Now that ST_Transform has an alignment parameter, you could easily remove the confusion in ST_Resample() by renaming _st_resample()to _st_gdalwarp() (what it truly is) and remove the SRID parameter from ST_Resample().

This way we have a set of usual functions each using the confusing GDAL Warp but avoiding the confusion. I don't know any other raster package mixing resampling and reprojecting in a unique function. I know reprojecting involves resampling but resampling certainly does not involve reprojecting.

It's not because GDAL is confusing that we have to do the same…

comment:11 by Bborie Park, 11 years ago

Sounds like a plan.

comment:12 by Bborie Park, 11 years ago

Keywords: history added
Resolution: fixed
Status: newclosed

Fixed in -trunk as of r10760

Note: See TracTickets for help on using tickets.