Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#25739 closed Cleanup/optimization (wontfix)

deprecate geos.factory.fromstr

Reported by: Sergey Fedoseev Owned by: Sergey Fedoseev
Component: GIS Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here's it's definition:

def fromstr(string, **kwargs):
    "Given a string value, returns a GEOSGeometry object."
    return GEOSGeometry(string, **kwargs)

This function is just an alias for GEOSGeometry, so I think it should be deprecated.

Change History (11)

comment:1 by Sergey Fedoseev, 8 years ago

Owner: changed from nobody to Sergey Fedoseev
Status: newassigned

comment:2 by Tim Graham, 8 years ago

Clearly someone thought there was a good reason for it at one point. Did you try looking through commit history and tickets to try to find if that reason is no longer applicable?

comment:3 by Sergey Fedoseev, 8 years ago

It looks like both GEOSGeometry and fromstr were introduced in this commit. Here's fromstr definition and here's definition of GEOSGeometry. At this point fromstr is just an alias for GEOSGeometry too.

comment:4 by Tim Graham, 8 years ago

While I agree two ways to do the same thing isn't ideal, I don't see much maintenance overhead in keeping the function. Unless there's a stronger rationale for the deprecation or other GIS contributors believe it's valuable to remove the function, I don't see much advantage to it.

comment:5 by Claude Paroz, 8 years ago

Resolution: wontfix
Status: assignedclosed

Let's keep it that way for now.

comment:6 by Sergey Fedoseev, 8 years ago

I think you underestimate it's maintenance overhead. Anyone who learns GIS internals will face this function and some time will be needed to understand that there is no benefit in using it.
Moreover this function is mentioned in documentation, so user can wrongly suppose that there is some advantage from using this function if geometry should be constructed from string.
In conclusion, even there's no much advantage in deprecation of this function, I don't see any disadvantage in it.

comment:7 by Tim Graham, 8 years ago

The disadvantage is projects having to update their code for the deprecation.

comment:8 by Claude Paroz, 8 years ago

What could be done is to lessen the prominence of fromstr in the docs. Only keep the method under Geometry Factories and add a note that it is equivalent to GEOSGeometry(string) or <GeometrySubclass>(string). Feel free to propose a docs patch.

comment:9 by Tim Graham <timograham@…>, 8 years ago

In 97e1d243:

Refs #25739 -- Lessened the prominence of geos.fromstr() in the docs.

comment:10 by Tim Graham <timograham@…>, 8 years ago

In 013309ea:

[1.9.x] Refs #25739 -- Lessened the prominence of geos.fromstr() in the docs.

Backport of 97e1d2433085e01696dc2fac8bfbb9c421c82b67 from master

comment:11 by Tim Graham <timograham@…>, 8 years ago

In 954d631:

[1.8.x] Refs #25739 -- Lessened the prominence of geos.fromstr() in the docs.

Backport of 97e1d2433085e01696dc2fac8bfbb9c421c82b67 from master

Note: See TracTickets for help on using tickets.
Back to Top