GeoTools

Clean up style deprecations introduced during migration to GeoAPI

Details

Description

A previous withdrawn proposal, Upgrading styles with GeoAPI Interfaces, introduced GeoAPI interfaces for Symbology Encoding into the GeoTools. This work was incomplete leaving us with many deprecated methods; and no alternative workaround.

This "proposal" (more case of technical dept) will clean up the style interfaces by:

  • Introduce read-write GeoTools interfaces for any new Symbology Encoding constructs
  • Restoring setters methods; the only deprecations will remain those where a clear alternative is available (example setTitle is now getDescription().setTitle )
  • TypeNarrow getters to be explicit about the GeoTools interface returned for any aggregations. Example: getDescription() will return the geotools Description interface
  • Completing the StyleFactoryImpl2 implementation; this is an implementation of geoapi style factory where the created results are type-narrowed to explicitly be the GeoTools interfaces
  • Updating StyleFactoryImpl to delegate to StyleFactoryImpl2 for the new "create" methods

While the GeoAPI interfaces will be left in the mix as super classes (providing a read-only interface) the GeoTools codebase will continue to make use of the GeoTools interfaces which will offer a read-write extension. This arrangement allows us to use the excellent work that went into producing Symbology Encoding 1.1 interfaces while preserving our current work flow.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jody Garnett added a comment -
Patch for 2.5.x; please review.
Show
Jody Garnett added a comment - Patch for 2.5.x; please review.
Hide
Permalink
Jody Garnett added a comment - - edited
To be clear; this proposal is to clean up all deprecations in the style interface (restoring all setter methods - not just the one example mentioned on the proposal page).

For GeoTools 2..5.x - we will do the bare minimum to (uncomment the setter methods;. I wsa forced to ype narrow a few things like getDescription() so that everything had setters to work with

For GeoTools 2.6.x - we will sort out a few more of the geoapi super classes
Show
Jody Garnett added a comment - - edited To be clear; this proposal is to clean up all deprecations in the style interface (restoring all setter methods - not just the one example mentioned on the proposal page). For GeoTools 2..5.x - we will do the bare minimum to (uncomment the setter methods;. I wsa forced to ype narrow a few things like getDescription() so that everything had setters to work with For GeoTools 2.6.x - we will sort out a few more of the geoapi super classes
Hide
Permalink
Andrea Aime added a comment -
Looked into the patch quickly, generally speaking looks good, I have some questions:
- ColorMapEntry was completely deprecated and now it's no more. What was (or still is) the substitute in 2.6.x?
- one thing that did strike me as completely wrong in the old setup was that in 2.5.x there were deprecations for stuff that could not be replaced with a non deprecated method. This is wrong, it makes it impossible to write code without deprecations. Even after the patch there are a few of those cases: ConstrastEnhancement.getType()
- why are we keeping Fill.ConstantFill around? Was it already there? There are quite a bit of these ConstantXXX around (mind, I'm just asking a reason, not asking to remove them)
- TextSymbolizer.getOption(key) could be depreated in favour of getOptions().get(key) (avoid two ways of doing the same thing in the interface)

I did not see StyleFactory2 around btw, is this part of the 2.6.x changes?
Also, OverlapBehaviour patch was rejected (and quite a few others were accepted with fuzz <= 2... not sure, maybe it's only due to different newlines).
Show
Andrea Aime added a comment - Looked into the patch quickly, generally speaking looks good, I have some questions: - ColorMapEntry was completely deprecated and now it's no more. What was (or still is) the substitute in 2.6.x? - one thing that did strike me as completely wrong in the old setup was that in 2.5.x there were deprecations for stuff that could not be replaced with a non deprecated method. This is wrong, it makes it impossible to write code without deprecations. Even after the patch there are a few of those cases: ConstrastEnhancement.getType() - why are we keeping Fill.ConstantFill around? Was it already there? There are quite a bit of these ConstantXXX around (mind, I'm just asking a reason, not asking to remove them) - TextSymbolizer.getOption(key) could be depreated in favour of getOptions().get(key) (avoid two ways of doing the same thing in the interface) I did not see StyleFactory2 around btw, is this part of the 2.6.x changes? Also, OverlapBehaviour patch was rejected (and quite a few others were accepted with fuzz <= 2... not sure, maybe it's only due to different newlines).
Hide
Permalink
Jody Garnett added a comment -
- ColorMapEntry: The color map stuff is now replaced by a function; there are a couple of "categorization" functions including one for color lookup. We may be able to "merge" these two ideas; so that the color lookup function can have its parameters edited using the colorentry api.
- Deprecations without replacement: if you find any of these let me know and we will fix them; ContrastEnhancement.getType is replaced by getMethod()
- I did not look at ConstantFill stuff; I was focused on deprecated set methods; the constants were generally there to capture default values; a responsibility we could pass off to a style builder and/or renderer
- We can deprecate TextSymbolizer.getOptions(key) as well; thanks for catching that

StyleFactory2 is part of the 2.6.x changes; indeed I would suggest we do not depend on the geoapi StyleFactoryInterface at all for 2.5.x.
Show
Jody Garnett added a comment - - ColorMapEntry: The color map stuff is now replaced by a function; there are a couple of "categorization" functions including one for color lookup. We may be able to "merge" these two ideas; so that the color lookup function can have its parameters edited using the colorentry api. - Deprecations without replacement: if you find any of these let me know and we will fix them; ContrastEnhancement.getType is replaced by getMethod() - I did not look at ConstantFill stuff; I was focused on deprecated set methods; the constants were generally there to capture default values; a responsibility we could pass off to a style builder and/or renderer - We can deprecate TextSymbolizer.getOptions(key) as well; thanks for catching that StyleFactory2 is part of the 2.6.x changes; indeed I would suggest we do not depend on the geoapi StyleFactoryInterface at all for 2.5.x.
Hide
Permalink
Jody Garnett added a comment -
Patch applied to 2.5.x as of r32906
Show
Jody Garnett added a comment - Patch applied to 2.5.x as of r32906
Hide
Permalink
Jody Garnett added a comment -
Fix applied to trunk as of -r32919; also picked up a few more things for 2.5.x as of -r32920
Show
Jody Garnett added a comment - Fix applied to trunk as of -r32919; also picked up a few more things for 2.5.x as of -r32920

People

  • Assignee:
    Jody Garnett
    Reporter:
    Jody Garnett
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: