Add a user/site specified background image directory #815

melissaebrooks · 2016-11-16T22:51:45Z

Issue #786 requests additional options for the stock image which would allow other images to be presented. Ideally new images would be available at a high enough resolution to allow plots over small areas to produce high quality images. For this to happen, the size of the raster data on the repository would have to increase to an inappropriate size.

To get around this, but still have the flexibility and speed of pre-downloaded and pre-resized images, I suggest having a directory of images defined by an environment variable which can be set to a site specifc default or changed by the user. That directory can hold an appropriate background images for multiple types of image, and resolution of the image. In order to define the contents of the image, I suggest it should contain a JSON file which sets up a dictionary of images by type and resolution.

To achieve fast plotting the I found it was necessary to add options to define the extent of the background image used in a plot, and the ability to cache the (whole) image into memory.

A tar file containing a set of images to do the Natural Earth Shaded Relief and Grey Earth, at a variety of resolutions, and the JSON file for them, is available at:
http://gws-access.ceda.ac.uk/public/mo_forecasts/natural_earth_backgrounds.tar

Code tested as an addition to Met Office cartopy install (which is currently non-standard as it uses some old dependencies).

marqh · 2016-12-20T11:23:35Z

Hi @malcolmbrooks

the master tests are now passing, which is a relief (#818)

the remaining test failure on this PR is a PEP8 failure

please may you
pep8 lib/cartopy/mpl/geoaxes.py

and fix the reported error

thank you
mark

Code tested as an addition to Met Office cartopy install (which is currently non-standard as it uses some old dependencies).

Conflicts: lib/cartopy/mpl/geoaxes.py

marqh · 2016-12-21T14:15:03Z

this is all passing now, which is grand

I am supportive of this new interface to share local data, it has some useful potential for performance and to help deployment

melissaebrooks · 2016-12-21T15:17:32Z

Thanks for your help @marqh

QuLogic

Generally reasonable, but this seems to box one in with a single directory (which granted, are able to contain multiple directories, I think). Is there a way to get the fall back to be <repo_data_dir>/raster/<name>? And additionally, <data_dir> (which is in user-space.)

QuLogic · 2016-12-22T07:21:25Z

lib/cartopy/data/raster/natural_earth/images.json

@@ -0,0 +1,7 @@
+{"__comment__": "JSON file specifying the image to use for a given type/name and resolution. Read in by cartopy.mpl.geoaxes.read_user_backgound_images.",


QuLogic · 2016-12-22T07:22:39Z

lib/cartopy/mpl/geoaxes.py

+    def background_img(self, name='ne_shaded', resln='low', extent=None,
+                       cache=False):
+        """
+        Adds from a selection of prepared images to the map where the


First sentence should be a single line.

Sorry, I'm not quite sure what you mean. pep8 compliance forces short lines here.

QuLogic · 2016-12-22T07:23:59Z

lib/cartopy/mpl/geoaxes.py

+                       cache=False):
+        """
+        Adds from a selection of prepared images to the map where the
+        available files are held in the CARTOPY_USER_BACKGROUNDS environment


CARTOPY_USER_BACKGROUNDS is a directory? This sentence implies it's a list of files.

I hope this is clearer now.

QuLogic · 2016-12-22T07:24:32Z

lib/cartopy/mpl/geoaxes.py

+        available files are held in the CARTOPY_USER_BACKGROUNDS environment
+        variable.
+
+        That directory is checked with read_user_backgound_images and needs to


backgound -> background; add back ticks for cross-reference.

QuLogic · 2016-12-22T07:24:48Z

lib/cartopy/mpl/geoaxes.py

+
+            * name - the name of the image to read according to the contents of
+                     the JSON file. A typical file might have, for instance:
+                     'ne_shaded' : Natrual Earth Shaded Relief


QuLogic · 2016-12-22T07:36:35Z

lib/cartopy/mpl/geoaxes.py

+        # Now obtain the image data from file or cache:
+        fpath = os.path.join(bgdir, fname)
+        if cache:
+            if fname in _BACKG_IMG_CACHE.keys():


.keys() is redundant.

QuLogic · 2016-12-22T07:41:33Z

lib/cartopy/mpl/geoaxes.py

+            # which points are in range:
+            lat_in_range = np.logical_and(lat_pts >= extent[2],
+                                          lat_pts <= extent[3])
+            if extent[0] < 180 and extent[1] > 180:


Would be nice if it could handle extent[1] that was negative.

That bit of logic catches extents that cross the dateline. An extent[1] that's negative doens't span the dateline so works in the else block.

QuLogic · 2016-12-22T07:42:45Z

lib/cartopy/mpl/geoaxes.py

+                               transform=source_proj,
+                               extent=ret_extent)
+
+    def read_user_backgound_images(self, verify=True):


QuLogic · 2016-12-22T07:45:27Z

lib/cartopy/mpl/geoaxes.py

+
+        The metadata should be a standard JSON file which specifies a two
+        level dictionary. The first level is the image type.
+        For each image type there needs to be the fields:


needs to -> must

QuLogic · 2016-12-22T07:47:27Z

lib/cartopy/mpl/geoaxes.py

+                    # check that this image type has the required info:
+                    for required in required_info:
+                        if required not in _USER_BG_IMGS[img_type]:
+                            err_str = 'User background metadata file'


I don't really like this +=ing stuff; just write the whole string and format it in one go.

I don't like it either but the pep8 compliance demands short lines, so it's a choice between useful error messages and += or useless error messages (or ignoring pep8 compliance).

Take advantage of other mechanisms for breaking lines, the += method makes the message unreadable in the source. Try something like this:

msg = ('User background metadata file "{}", image type "{}", does not ' 'specify metadata item {}') raise ValueError(msg.format(json_file, img_type, required))

Yes, that would be much better.

QuLogic · 2016-12-22T07:58:22Z

And additionally, <data_dir> (which is in user-space.)

Though, since this exists, do we really need the environment variable at all?

marqh · 2016-12-28T13:55:02Z

And additionally, <data_dir> (which is in user-space.)

Though, since this exists, do we really need the environment variable at all?

There seems to me a significant difference here.

With this environment variable, I can specify, at run time, a pre-prepared selection of data, ready to use. This has significant benefits in a shared environment, where cartographic data sets can be prepared in advance, then used by multiple systems in an optimised fashion.

This is a useful extension use case, which I don't think is covered by data_dir or individual user space.

melissaebrooks · 2016-12-29T11:33:33Z

Thanks for the read-through @QuLogic
Your typo spotting is much appreciated.

…o background_img Conflicts: lib/cartopy/mpl/geoaxes.py

ajdawson · 2016-12-29T15:46:10Z

lib/cartopy/mpl/geoaxes.py

+        try:
+            fname = _USER_BG_IMGS[name][resolution]
+        except KeyError:
+            msg = ('Image "{} and "resolution "{}" are not present in '


Looks like the " is in the wrong place here.

ajdawson · 2016-12-29T18:50:56Z

lib/cartopy/mpl/geoaxes.py

+        try:
+            fname = _USER_BG_IMGS[name][resolution]
+        except KeyError:
+            msg = ('Image "{}" and "resolution "{}" are not present in '


Do you need the " before resolution? It doesn't have a corresponding closing quote.

melissaebrooks

The change is now passing tests, apart from the python 3.6 one which doesn't have anything to do with this change. The first of the recent changes also failed it, and that only made a change to the comment in the JSON file.

The environment variable declaring a directory does fall back to the repo dir if it's unset. That's what is done in the tests.

melissaebrooks · 2016-12-29T15:25:03Z

lib/cartopy/mpl/geoaxes.py

+                    # check that this image type has the required info:
+                    for required in required_info:
+                        if required not in _USER_BG_IMGS[img_type]:
+                            err_str = 'User background metadata file'


Yes, that would be much better.

ajdawson · 2016-12-30T11:57:23Z

The pep8 error is your code change though.

Merge branch 'background_img' of github.com:malcolmbrooks/cartopy into background_img Conflicts: lib/cartopy/mpl/geoaxes.py

melissaebrooks · 2017-01-03T12:53:50Z

Thanks @ajdawson; the change is now passing tests, apart from the python 3.6 one which doesn't have anything to do with this change. The first of the recent changes also failed it, and that only made a change to the comment in the JSON file.

marqh · 2017-01-06T15:06:57Z

Thanks @ajdawson; the change is now passing tests, apart from the python 3.6 one which doesn't have anything to do with this change.

@malcolmbrooks you're right, not your problem; it is now fixed, any future commit should not suffer from this

marqh · 2017-01-10T15:25:51Z

this is all looking good to me now, I think there has been plenty of input

I'm merging this, thank you @malcolmbrooks

Malcolm Brooks added 4 commits November 16, 2016 21:52

Added code to include user defined backgroudn image library.

a8d86f8

Code tested as an addition to Met Office cartopy install (which is currently non-standard as it uses some old dependencies).

Docstring and pylint tweaks

6508da5

comment tidyup

8110685

Added more detail to docstring

70e5233

Malcolm Brooks added 9 commits December 20, 2016 13:36

Added code to include user defined backgroudn image library.

b67d1e8

Code tested as an addition to Met Office cartopy install (which is currently non-standard as it uses some old dependencies).

Docstring and pylint tweaks

d99867c

comment tidyup

e1df367

Added more detail to docstring

3259554

Added the background_img code again.

dd63e87

Conflicts: lib/cartopy/mpl/geoaxes.py

pep8 'readability' changes

2a8b846

Filename fix

b9a114e

One last pep8 compliance test with ugly workaround

6065e33

pep8 wants two lines between tests

e2e001b

melissaebrooks force-pushed the background_img branch from d7a2701 to e2e001b Compare December 20, 2016 13:50

Changed test to refer to per-existing image

327b035

QuLogic reviewed Dec 22, 2016

View reviewed changes

Malcolm Brooks added 5 commits December 29, 2016 10:34

Update images.json

d8c888d

Update geoaxes.py

5f3d108

Update test_images.py

d17863a

Update geoaxes.py

6e022b5

Update geoaxes.py

46bfacd

Malcolm Brooks added 3 commits December 29, 2016 15:22

Update geoaxes.py

3dbbfaa

Merge branch 'background_img' of github.com:malcolmbrooks/cartopy int…

0c5f85c

…o background_img Conflicts: lib/cartopy/mpl/geoaxes.py

Update geoaxes.py

f6badcc

ajdawson reviewed Dec 29, 2016

View reviewed changes

Update geoaxes.py

b6cb0f1

ajdawson reviewed Dec 29, 2016

View reviewed changes

Update geoaxes.py

905a160

melissaebrooks commented Dec 30, 2016

View reviewed changes

Malcolm Brooks added 2 commits January 3, 2017 11:31

Pep8 conformance

ec0d636

Merge branch 'background_img' of github.com:malcolmbrooks/cartopy into background_img Conflicts: lib/cartopy/mpl/geoaxes.py

License needs to include year of change

f85b8cc

Update geoaxes.py

580e965

marqh merged commit bf37d53 into SciTools:master Jan 10, 2017

melissaebrooks mentioned this pull request Jan 12, 2017

0.15 release #838

Closed

marqh mentioned this pull request Feb 2, 2017

Add whats new for 0.15.0 release #846

Merged

QuLogic added this to the 0.15.0 milestone Jun 24, 2017

pelson mentioned this pull request Nov 20, 2017

Additional options for the stock image #786

Closed

ocefpaf mentioned this pull request Jun 7, 2019

Paper: PyDDA: A New Pythonic Multiple Doppler Retrieval Package scipy-conference/scipy_proceedings#474

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a user/site specified background image directory #815

Add a user/site specified background image directory #815

melissaebrooks commented Nov 16, 2016

marqh commented Dec 20, 2016

marqh commented Dec 21, 2016

melissaebrooks commented Dec 21, 2016

QuLogic left a comment

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

melissaebrooks Dec 29, 2016

QuLogic Dec 22, 2016

melissaebrooks Dec 29, 2016

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

melissaebrooks Dec 29, 2016

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

QuLogic Dec 22, 2016

melissaebrooks Dec 29, 2016

ajdawson Dec 29, 2016

melissaebrooks Dec 29, 2016

QuLogic commented Dec 22, 2016

marqh commented Dec 28, 2016

melissaebrooks commented Dec 29, 2016

ajdawson Dec 29, 2016

ajdawson Dec 29, 2016

melissaebrooks left a comment

melissaebrooks Dec 29, 2016

ajdawson commented Dec 30, 2016

melissaebrooks commented Jan 3, 2017

marqh commented Jan 6, 2017

marqh commented Jan 10, 2017

		@@ -0,0 +1,7 @@
		{"__comment__": "JSON file specifying the image to use for a given type/name and resolution. Read in by cartopy.mpl.geoaxes.read_user_backgound_images.",

Add a user/site specified background image directory #815

Add a user/site specified background image directory #815

Conversation

melissaebrooks commented Nov 16, 2016

marqh commented Dec 20, 2016

marqh commented Dec 21, 2016

melissaebrooks commented Dec 21, 2016

QuLogic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuLogic commented Dec 22, 2016

marqh commented Dec 28, 2016

melissaebrooks commented Dec 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melissaebrooks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajdawson commented Dec 30, 2016

melissaebrooks commented Jan 3, 2017

marqh commented Jan 6, 2017

marqh commented Jan 10, 2017