[Thuban-devel] Re: RfD: Support for layer specific resources

[Bernhard Herzog]

Martin Schulze <joey at infodrom.org> writes:

> Bernhard Herzog wrote:
>> No.  The code outside of Thuban.UI should not depend on the GUI.
>
> Hm, that means that we'll have to move the properties and information
> dialogs from Extensions/wms into Thuban/UI in the long-term.


Well, I should have phrased that perhaps a bit more strictly: The code
under Thuban but outside of Thuban.UI should not depend on Thuban.UI.
Extensions are a different matter.  How they are organized is up to each
extension more or less, depending mostly on the maturity of the
extension.  It's a good idea in general to keep GUI code separate from
the application logic so in Extensions the GUI and logic should at least
be in separate modules, but for very small or experimental extensions
it's often not a problem to mix them.


>> I'd prefer a solution like the one used for the renderer extensions in
>> baserenderer.py.  It basically means that there's a mapping between
>> layer classes and property dialog classes.  This mapping should be in a
>> new module, I think, together with the GeneralPropertiesDialog class.
>> The OpenLayerProperties method would call a function in that module
>> which returns the dialog class for a given layer.
>
> Understood.
>
> Is the patch below and the two new files (test/test_layer_dialogs.py
> and Thuban/UI/layer_dialogs.py) what you want?

Not quite.  See below, especially my comment on open_properties_dialog.


[2. text/x-python; test_layer_dialogs.py]
[...]
> class TestLayerDialogs(unittest.TestCase):
>
>     def test_layer_dialogs(self):
>
>         class MyLayer(BaseLayer):
>             pass
>
>         my_class = MyLayer("test")

Why "my_class"?  It's not a class.



[3. text/x-python; layer_dialogs.py]
> def open_properties_dialog(parent, layer, group):
>     for cls, func in _layer_dialogs['properties']:
>         if isinstance(layer, cls):
>             func(parent, layer, group)
>             return
>     raise NotImplementedError

This function should return the class, not the already instantiated
dialog.  More precisely, the return value should be an object that when
called with name, parent and layer creates the dialog and returns it.
If there is not dialog class available for a given layer, the function
should return None.

Also, the function shouldn't be called open_properties_dialog then, but
perhaps get_layer_properties_dialog instead.

The logic that determines whether the dialog is already shown and either
raises the existing one or creates a new one should be left in the
mainwindow.  Actually, it should be moved into its own mainwindow method
as this logic has already been duplicated at least seven times in
mainwindow.py, but that's for another patch.


> Index: mainwindow.py
> ===================================================================
> RCS file: /thubanrepository/thuban/Thuban/UI/mainwindow.py,v
> retrieving revision 1.132
> diff -u -r1.132 mainwindow.py
[...]
> @@ -607,6 +610,10 @@
>          """Return true if a shape layer is currently selected"""
>          return isinstance(self.current_layer(), Layer)
>  
> +    def has_selected_wms_layer(self):
> +        """Return true if a WMS layer is currently selected"""
> +        return 'capabilities' in dir(self.current_layer())

This is specific for one extension.  Such code should not be in the
Thuban core, i.e. the code in Thuban/.  Also, you use it to determine
whether the sensitivity of the layer_show_table command, which AFAICT
doesn't make sense for WMS layers.  If that's a bug and you wanted to
use it for the sensitivity of the layer_properties command, then it's
better to have a method that returns whether a dialog can be opened for
the currently selected layer, i.e. whether get_layer_properties_dialog
returns None or not.


   Bernhard

-- 
Intevation GmbH                                 http://intevation.de/
Skencil                                http://sketch.sourceforge.net/
Thuban                                  http://thuban.intevation.org/