Skip to content

Perceptual image hashing #2206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marqh merged 2 commits into from
Oct 24, 2016
Merged

Perceptual image hashing #2206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb4581e
Perceptual image hashing.
bjlittle Oct 15, 2016
3bc2a1f
Ensure thread-safe plotting.
bjlittle Oct 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ install:
fi
fi

# Perceptual image hashing (TBD: push recipe to conda-forge!)
- conda install pip
Copy link
Member

@DPeterK DPeterK Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW you don't need to install pip - it's already available on Travis.

Copy link
Member

@QuLogic QuLogic Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of the conda environment (and installing to it), though?

Copy link
Member

@ajdawson ajdawson Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip is installed by default when you create a new conda environment (e.g. https://travis-ci.org/SciTools/iris/jobs/170017955#L253) so it doesn't need to be done separately. The result of this line is a no-op (https://travis-ci.org/SciTools/iris/jobs/170017955#L464).

Copy link
Member Author

@bjlittle bjlittle Oct 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

- pip install imagehash

- PREFIX=$HOME/miniconda/envs/$ENV_NAME

# Output debug info
Expand Down
5 changes: 2 additions & 3 deletions docs/iris/example_tests/extest_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from iris._deprecation import IrisDeprecation
import iris.plot as iplt
import iris.quickplot as qplt
from iris.tests import _DEFAULT_IMAGE_TOLERANCE


EXAMPLE_DIRECTORY = os.path.join(os.path.dirname(os.path.dirname(__file__)),
Expand All @@ -60,7 +59,7 @@ def add_examples_to_path():


@contextlib.contextmanager
def show_replaced_by_check_graphic(test_case, tol=_DEFAULT_IMAGE_TOLERANCE):
def show_replaced_by_check_graphic(test_case):
"""
Creates a context manager which can be used to replace the functionality
of matplotlib.pyplot.show with a function which calls the check_graphic
Expand All @@ -69,7 +68,7 @@ def show_replaced_by_check_graphic(test_case, tol=_DEFAULT_IMAGE_TOLERANCE):
"""
def replacement_show():
# form a closure on test_case and tolerance
test_case.check_graphic(tol=tol)
test_case.check_graphic()
Copy link
Member Author

@bjlittle bjlittle Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the hope of refraining from a culture of tweaking graphical testing tolerances, I've closed the door to passing a tol through to check_graphic.


orig_show = plt.show
plt.show = iplt.show = qplt.show = replacement_show
Expand Down
2 changes: 1 addition & 1 deletion docs/iris/example_tests/test_atlantic_profiles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_atlantic_profiles(self):
with fail_any_deprecation_warnings():
with add_examples_to_path():
import atlantic_profiles
with show_replaced_by_check_graphic(self, tol=14.0):
with show_replaced_by_check_graphic(self):
Copy link
Member Author

@bjlittle bjlittle Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This covers up an issue in the use of twiny ... changing the tolerance of an individual test is not healthy.

atlantic_profiles.main()


Expand Down
35 changes: 34 additions & 1 deletion lib/iris/plot.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

import collections
import datetime
from functools import wraps
import threading

import cartopy.crs as ccrs
import cartopy.mpl.geoaxes
Expand All @@ -52,9 +54,28 @@
# Cynthia Brewer citation text.
BREWER_CITE = 'Colours based on ColorBrewer.org'


PlotDefn = collections.namedtuple('PlotDefn', ('coords', 'transpose'))

# Threading reentrant lock to ensure thread-safe plotting.
_lock = threading.RLock()


def _locker(func):
"""
Decorator that ensures a thread-safe atomic operation is
performed by the decorated function.

Uses a shared threading reentrant lock to provide thread-safe
plotting by public API functions.

"""
@wraps(func)
def decorated_func(*args, **kwargs):
with _lock:
result = func(*args, **kwargs)
return result
return decorated_func

Copy link
Member Author

@bjlittle bjlittle Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple decorator to facilitate the use of a re-entrant lock to provide thread-safe plotting. A re-entrant lock is required here as iris.plot.contourf calls iris.plot.contour ...


def _get_plot_defn_custom_coords_picked(cube, coords, mode, ndims=2):
def names(coords):
Expand Down Expand Up @@ -665,6 +686,7 @@ def _map_common(draw_method_name, arg_func, mode, cube, plot_defn,
return plotfn(*new_args, **kwargs)


@_locker
def contour(cube, *args, **kwargs):
Copy link
Member Author

@bjlittle bjlittle Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the lock for all public api plotting functions ...

Copy link
Member

@DPeterK DPeterK Oct 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether this will have any unexpected behavioural impacts in normal usage of iris.plot? I must admit that I expected that the lock would be applied within the scope of the tests only.

Copy link
Member

@ajdawson ajdawson Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to agree... it feels like this should be done when testing. We are mostly wrapping matplotlib calls, which I guess aren't thread safe either, so it doesn't seem consistent to lock our wrappers.

Copy link
Member Author

@bjlittle bjlittle Oct 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkillick @ajdawson I see your point, and I was in two minds whether to lock at this level. To be honest, I expected that discussion to have happen during the review 😉 ...

Regardless of testing, iris plotting is not thread safe for users thanks to mpl, hence why I locked the wrappers at the plotting level. The problem lies deep within iris plotting regarding calls to iris.plot._replace_axes_with_cartopy_axes

Locking at the graphics test level completely makes sense as it totally ensures that graphics tests are atomic (as I've already implemented), so at a minimum we definitely require to keep that level of locking. The plotting level of locking may help the user in a threaded context, but I'm less convinced of this ... So we may be able to compromise and drop the plotting level of locking ... I'll take a look and see how it behaves. But I'd be more than happy to do that.

"""
Draws contour lines based on the given Cube.
Expand All @@ -689,6 +711,7 @@ def contour(cube, *args, **kwargs):
return result


@_locker
def contourf(cube, *args, **kwargs):
"""
Draws filled contours based on the given Cube.
Expand Down Expand Up @@ -815,6 +838,7 @@ def _fill_orography(cube, coords, mode, vert_plot, horiz_plot, style_args):
return result


@_locker
def orography_at_bounds(cube, facecolor='#888888', coords=None, axes=None):
"""Plots orography defined at cell boundaries from the given Cube."""

Expand Down Expand Up @@ -845,6 +869,7 @@ def horiz_plot(v_coord, orography, style_args):
horiz_plot, style_args)


@_locker
def orography_at_points(cube, facecolor='#888888', coords=None, axes=None):
"""Plots orography defined at sample points from the given Cube."""

Expand All @@ -866,6 +891,7 @@ def horiz_plot(v_coord, orography, style_args):
horiz_plot, style_args)


@_locker
def outline(cube, coords=None, color='k', linewidth=None, axes=None):
"""
Draws cell outlines based on the given Cube.
Expand Down Expand Up @@ -903,6 +929,7 @@ def outline(cube, coords=None, color='k', linewidth=None, axes=None):
return result


@_locker
def pcolor(cube, *args, **kwargs):
"""
Draws a pseudocolor plot based on the given Cube.
Expand All @@ -929,6 +956,7 @@ def pcolor(cube, *args, **kwargs):
return result


@_locker
def pcolormesh(cube, *args, **kwargs):
"""
Draws a pseudocolor plot based on the given Cube.
Expand All @@ -953,6 +981,7 @@ def pcolormesh(cube, *args, **kwargs):
return result


@_locker
def points(cube, *args, **kwargs):
"""
Draws sample point positions based on the given Cube.
Expand Down Expand Up @@ -980,6 +1009,7 @@ def _scatter_args(u, v, data, *args, **kwargs):
*args, **kwargs)


@_locker
def plot(*args, **kwargs):
"""
Draws a line plot based on the given cube(s) or coordinate(s).
Expand Down Expand Up @@ -1024,6 +1054,7 @@ def plot(*args, **kwargs):
return _draw_1d_from_points('plot', _plot_args, *args, **kwargs)


@_locker
def scatter(x, y, *args, **kwargs):
"""
Draws a scatter plot based on the given cube(s) or coordinate(s).
Expand Down Expand Up @@ -1059,6 +1090,7 @@ def scatter(x, y, *args, **kwargs):
show = plt.show


@_locker
def symbols(x, y, symbols, size, axes=None, units='inches'):
"""
Draws fixed-size symbols.
Expand Down Expand Up @@ -1122,6 +1154,7 @@ def symbols(x, y, symbols, size, axes=None, units='inches'):
axes.autoscale_view()


@_locker
def citation(text, figure=None, axes=None):
"""
Add a text citation to a plot.
Expand Down
Loading