Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#1994 closed task (fixed)

Deprecate all functions with underscore_names rather than CamelCase

Reported by: strk Owned by: pramsey
Priority: medium Milestone: PostGIS 2.1.0
Component: postgis Version: master
Keywords: Cc:

Description

All functions in core should be ST_CamelCase, so 2.1 should provide the renames of leftover under_scored and deprecate them.

I've no time for listing them here _now_ but I remember all the force_dims are underscored.

Change History (18)

comment:1 by strk, 12 years ago

What's the deprecation policy ?

  1. We add the new signature
  2. We mark the old signature as deprecated in the SQL
  3. How do we document ? Do we drop documentation for old syntax and keep the new one ?
  4. We test both signatures

comment:2 by strk, 12 years ago

See if you like what I did for ST_EstimatedExtent in r10279. I've to say I'd still like deprecated functions to emit a WARNING about it, but we never did it before…

comment:3 by strk, 12 years ago

Leftover list, obtained by extracting from documentation files the lines matching the regular expression 'function>ST_.*_' ignoring case:

  • ST_Force_2D
  • ST_Force_3D
  • ST_Force_3DZ
  • ST_Force_3DM
  • ST_Force_4D
  • ST_Force_Collection
  • ST_Line_Interpolate_Point
  • ST_Line_Locate_Point
  • ST_Line_Substring
  • ST_Distance_Sphere
  • ST_Distance_Spheroid
  • ST_Length_Spheroid
  • ST_Length2D_Spheroid
  • ST_3DLength_Spheroid
  • ST_Mem_Size
  • ST_Point_Inside_Circle
  • ST_Shift_Longitude

comment:4 by robe, 12 years ago

policy. I don't think we have an official one, though we should have one.

My policy anyway:

1) Add new signature 2) Mark old signature as deprecated in SQL 3) We completely drop the documentation of old signature so that new users don't know it ever existed and old users won't see descriptions next to the function so they should get the hint hopefully not to use it 4) Drop testing old signature and replace with new signature 5) in 1-2 years later, move the deprecated functions to the legacy file unless we feel these functions were so stupid or new that it is not likely anyone used them. In that case we move them right away before anyone starts using them.

I think though that all the above are probably in use so follow the 1-2 year rule. ESPECIALLY the linear referencing ones.

The ST_Mem_Size, ST_Point_Inside_Circle, ST_Shift_Longitude (iffy since it's new but probably a somewhat popular function for those who have to deal with google/bing map stuff) you might be able to get away with moving into the legacy file now.

comment:5 by strk, 12 years ago

r11190 deprecates ST_Line_Interpolate_Point, ST_Line_Substring and ST_Line_Locate_Point. r11191 adds an actual deprecation warning for these and for ST_Estimated_Extent.

Leftovers:

  • ST_Shift_Longitude(geometry)
  • ST_Combine_BBox(box2d,geometry)
  • ST_find_extent(text,text,text) RETURNS box2d AS
  • ST_find_extent(text,text) RETURNS box2d AS
  • ST_mem_size(geometry)
  • ST_3DLength_spheroid(geometry, spheroid)
  • ST_length_spheroid(geometry, spheroid)
  • ST_length2d_spheroid(geometry, spheroid)
  • ST_distance_spheroid(geom1 geometry, geom2 geometry,spheroid)
  • ST_point_inside_circle(geometry,float8,float8,float8)
  • ST_force_2d(geometry)
  • ST_force_3dz(geometry)
  • ST_force_3d(geometry)
  • ST_force_3dm(geometry)
  • ST_force_4d(geometry)
  • ST_force_collection(geometry)
  • ST_locate_between_measures(geometry, float8, float8)
  • ST_locate_along_measure(geometry, float8)
  • ST_Combine_BBox(box3d,geometry)
  • ST_distance_sphere(geom1 geometry, geom2 geometry)
  • ST_distance_sphere(geom1 geometry, geom2 geometry)

comment:6 by robe, 12 years ago

Yah we need to keep them around for another 2 years. I would just get rid of them from the docs. Too many to say "deprecated looky here instead".

comment:7 by robe, 12 years ago

Milestone: PostGIS 2.1.0PostGIS 2.2.0

Please let us not deprecate anymore in this release. I've already got issues with those we've got deprecated producing annoying noise.

comment:8 by strk, 12 years ago

NOTE: r11406 (in time for 2.1.0) renamed all of ST_Force_XXX to ST_ForceXXX

Leftovers:

ST_Shift_Longitude(geometry) ST_Combine_BBox(box2d,geometry) ST_find_extent(text,text,text) RETURNS box2d AS ST_find_extent(text,text) RETURNS box2d AS ST_mem_size(geometry) ST_3DLength_spheroid(geometry, spheroid) ST_length_spheroid(geometry, spheroid) ST_length2d_spheroid(geometry, spheroid) ST_distance_spheroid(geom1 geometry, geom2 geometry,spheroid) ST_point_inside_circle(geometry,float8,float8,float8) ST_locate_between_measures(geometry, float8, float8) ST_locate_along_measure(geometry, float8) ST_Combine_BBox(box3d,geometry) ST_distance_sphere(geom1 geometry, geom2 geometry) ST_distance_sphere(geom1 geometry, geom2 geometry)

comment:9 by strk, 12 years ago

Correctly formatted now:

    ST_Shift_Longitude(geometry)
    ST_Combine_BBox(box2d,geometry)
    ST_find_extent(text,text,text) RETURNS box2d AS
    ST_find_extent(text,text) RETURNS box2d AS
    ST_mem_size(geometry)
    ST_3DLength_spheroid(geometry, spheroid)
    ST_length_spheroid(geometry, spheroid)
    ST_length2d_spheroid(geometry, spheroid)
    ST_distance_spheroid(geom1 geometry, geom2 geometry,spheroid)
    ST_point_inside_circle(geometry,float8,float8,float8)
    ST_locate_between_measures(geometry, float8, float8)
    ST_locate_along_measure(geometry, float8)
    ST_Combine_BBox(box3d,geometry)
    ST_distance_sphere(geom1 geometry, geom2 geometry)
    ST_distance_sphere(geom1 geometry, geom2 geometry) 

comment:10 by robe, 12 years ago

Milestone: PostGIS 2.2.0PostGIS 2.1.0

you have any more left? You are keeping the old as well right?

comment:11 by colivier, 12 years ago

I've seen that regress/wmsserver have been touched related to this one. Do you feel OK with such a change ?

As it doesn't reflect real MapServer/GeoServer PostGIS calls…

comment:12 by robe, 12 years ago

colivier,

Thanks for pointing that out. I think strk should revert his change. As the point of that regress is to make sure we don't break anything we know is currently being used.

comment:13 by strk, 12 years ago

Yes, sorry I'll revert the regress changes. The old signatures are still available, only they also spit a WARNING about them being deprecated. I decided to change the calls rather than either expect the WARNINGs or hush them. We want to test that both old and new work, should I duplicate the tests, then ?

comment:14 by strk, 12 years ago

How about r11457 ? Do you like it ? I just ignore WARNING in wmsservers.sql, keeping the renames in the other regression tests.

Robe all the left ones are written a few comments above. I'm doing this on my spare time so since beta is out already we can stop here.

comment:15 by robe, 12 years ago

Resolution: fixed
Status: newclosed

Okay that looks fine. So lets close this one out and start a new one for 2.2.

comment:16 by realityexists, 11 years ago

I'm getting a lot of these warnings for deprecated functions logged now and I'm not sure it would be safe to move to the new function names, since not all databases running my code my have upgraded to 2.1.

It's too late now for 2.1, but in future it would be good to get an "in-between" release for such changes that provides both old and new function names without a warning. So for example 2.2 adds a new function name and documents that the old one is deprecated, but doesn't log a warning. 2.3 then adds a warning. This gives users time to change their code more safely.

comment:17 by robe, 11 years ago

realityexists,

I couldn't agree with you more. Can you write up a separate ticket for it and slate to 2.1.1. I have to say I had mixed feelings about these warnings and when I saw the junk spewing from my tiger geocoder which I need on 2.0 dbs (and issue pgRouting was having) as well I was more leaning against this.

Technically we CAN change this in 2.1.1 since it doesn't change the API at all, and while not quite a bug — I would put it high up on the ANNOYING list.

Then we can put it back in 2.2 since 2.1 and 2.2 would then have the new functions.

I'm suspecting we'll have a 2.1.1 out in under a month since there were some bugs I just had to push to 2.1.1 because I couldn't find a way to recreate and they didn't look that serious.

comment:18 by realityexists, 11 years ago

OK, ticket #2440 opened.

Note: See TracTickets for help on using tickets.