Opened 7 years ago

Closed 6 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: trunk
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 7 years ago.
Attempt at using special windows functions for Win64 case…

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by pracine

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 Changed 7 years ago by robe

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 Changed 7 years ago by pracine

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 Changed 7 years ago by pracine

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.

Changed 7 years ago by pramsey

Attachment: sb.patch added

Attempt at using special windows functions for Win64 case...

comment:5 Changed 7 years ago by pramsey

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 Changed 7 years ago by Bborie Park

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

comment:7 Changed 7 years ago by robe

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 Changed 7 years ago by pracine

Milestone: PostGIS 2.0.0PostGIS 2.1.0

Then I postpone it to 2.1...

comment:9 Changed 6 years ago by Bborie Park

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 Changed 6 years ago by pracine

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 Changed 6 years ago by Bborie Park

Sounds like a plan.

comment:12 Changed 6 years ago by Bborie Park

Keywords: history added
Resolution: fixed
Status: newclosed

Fixed in -trunk as of r10760

Note: See TracTickets for help on using tickets.