GeoTools
  1. GeoTools
  2. GEOT-2468

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

        Hide
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: