Fixes issue 255

[fromon]

In master from revision 1.0.0 the method write_geotiffimage has been creating the files in gray scale even if colormap has a different value the objects metadata. This change first checks the metadata and only falls back to gray if it can't use the colormap given. (As is stated in the docstring.)

[coveralls]

Coverage decreased (-0.3%) to 93.506% when pulling b27ed2e82c40e501535127d772b8bcfb532161e1 on fromon:issue255_color_geotiff_writer into b11d8d6a4e5ed2cb4644ee4defb5d4ce635d5dc2 on nansencenter:master.

[akorosov]

@fromon , thank you for the fix!

It looks OK, however matplotlib is not a strict requirement of Nansat. We usually use try/except to import matlab and then set global MATPLOTLIB_INSTALLED to False if it is not. See, for example, https://github.com/nansencenter/nansat/blob/b11d8d6a4e5ed2cb4644ee4defb5d4ce635d5dc2/nansat/tools.py#L24

Can you please add a similar behaviour to nansat.py?

[akorosov]

Thank you @fromon, for the changes! generally I prefer not to use try/except when a simple check can be done. Besides it is recommended to put only one statement inside try and always catch a specific exception. What if you rewrite it into:

        colormap = band.GetMetadataItem('colormap')
        if MATPLOTLIB_IS_INSTALLED:
            cmap = cm.get_cmap(colormap, 256)
            cmap = cmap(np.arange(256)) * 255
        else:
            self.logger.debug('Geotiff is only available in gray '
                                  'since matplotlib is not installed.')
            cmap = np.vstack([np.arange(256.),
                              np.arange(256.),
                              np.arange(256.),
                              np.ones(256)*255]).T

I would have suggested that myself right in the code but I don't have access to your fork.

[fromon]

I am not sure you can write it like that, because if I understand Nansat correctly then a band does not have to contain a colormap, and GetMetadataItem() will raise an error if it doesn't. So the first line has to be in a try block. And the colormap is only a string, so it can be a string that cm.getmap() doesn't recognize. So the even if Matplotlib is installed it can still fail. That is why I think the command cmap = cm.get_cmap... should also be in a try block. and since both issues + if Matplotlib is not installed, all should result in the same fallback solution, I think my suggestion results in the cleanest code. But do correct me if I have misunderstood anything.