Skip to content
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

Jump to bottom

Add an origin keyword to aperture plot methods #395

Merged
merged 3 commits into from Jul 20, 2016
Merged

Add an origin keyword to aperture plot methods #395

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17ef1b9
Add a _prepare_plot method for plotting apertures and an origin keyword
larrybradley Jul 19, 2016
5f6712f
Use _prepare_plot in all plot methods
larrybradley Jul 19, 2016
6092f70
Add changelog entry [skip-ci]
larrybradley Jul 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
175 changes: 96 additions & 79 deletions photutils/aperture/core.py
View file
Open in desktop
Expand Up @@ -108,13 +108,76 @@ class PixelAperture(Aperture):
optionally, ``area``).
"""

def _prepare_plot(self, origin=(0, 0), source_id=None, ax=None,
fill=False, **kwargs):
"""
Prepare to plot the aperture(s) on a matplotlib Axes instance.

Parameters
----------
origin : array-like, optional
The ``(x, y)`` position of the origin of the displayed
image.

Copy link
Member

Choose a reason for hiding this comment

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

Would you like to add e.g. where``(x,y)``corresponds to the usual rectangular coordinate system? (if that's the case).

Copy link
Member Author

Choose a reason for hiding this comment

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

What other coordinate system did you have in mind? I think it's pretty clear that (x, y) means pixel coordinates if that's what you mean.

source_id : int or array of int, optional
The source ID(s) of the aperture(s) to plot.

ax : `matplotlib.axes.Axes` instance, optional
If `None`, then the current ``Axes`` instance is used.

fill : bool, optional
Set whether to fill the aperture patch. The default is
`False`.

kwargs
Any keyword arguments accepted by `matplotlib.patches.Patch`.

Returns
-------
plot_positions : `~numpy.ndarray`
The positions of the apertures to plot, after any
``source_id`` slicing and origin shift.

ax : `matplotlib.axes.Axes` instance, optional
The `matplotlib.axes.Axes` on which to plot.

kwargs
Any keyword arguments accepted by `matplotlib.patches.Patch`.
"""

import matplotlib.pyplot as plt

if ax is None:
ax = plt.gca()

# This is necessary because the `matplotlib.patches.Patch` default
# is ``fill=True``. Here we make the default ``fill=False``.
kwargs['fill'] = fill

plot_positions = copy.deepcopy(self.positions)
if source_id is not None:
plot_positions = plot_positions[np.atleast_1d(source_id)]

plot_positions[:, 0] -= origin[0]
plot_positions[:, 1] -= origin[1]

return plot_positions, ax, kwargs

@abc.abstractmethod
def plot(self, ax=None, fill=False, **kwargs):
def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
Copy link
Member

Choose a reason for hiding this comment

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

Could we maybe keep the original order of keyword args, in case someone used them as positional in their code?

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking was to group the matplotlib-specific keywords together (ax, fill, **kwargs) and that meant at the end because **kwargs has to be last. We've said we'll change the API to improve things, but I'm happy to change it back to (ax, fill, origin, source_id, **kwargs) if you think a lot of people are using this function and are using positional arguments.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with it if we said that API changes are expected (and actually like your logic).
TBH I have no idea how big is our user base, especially the one who doesn't use tools as they should, so probably this won't be a problem anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, thanks. I'll leave it as is. I don't know how big our user base is either, but it seems to be growing, and more people are asking me about the package.

**kwargs):
"""
Plot the aperture(s) on a matplotlib Axes instance.

Parameters
----------
origin : array-like, optional
The ``(x, y)`` position of the origin of the displayed
image.

source_id : int or array of int, optional
The source ID(s) of the aperture(s) to plot.

ax : `matplotlib.axes.Axes` instance, optional
If `None`, then the current ``Axes`` instance is used.

Expand Down Expand Up @@ -316,21 +379,15 @@ def __init__(self, positions, r):
def area(self):
return math.pi * self.r ** 2

def plot(self, ax=None, fill=False, source_id=None, **kwargs):
def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

if ax is None:
ax = plt.gca()
if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]

for position in positions:
for position in plot_positions:
patch = mpatches.Circle(position, self.r, **kwargs)
ax.add_patch(patch)

Expand Down Expand Up @@ -489,26 +546,16 @@ def get_fractions(self, data, method='exact', subpixels=5):
subpixels=subpixels, r_in=self.r_in)
return fractions

def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):


def plot(self, ax=None, fill=False, source_id=None, **kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill

if ax is None:
ax = plt.gca()

if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

resolution = 20

for position in positions:
for position in plot_positions:
patch_inner = mpatches.CirclePolygon(position, self.r_in,
resolution=resolution)
patch_outer = mpatches.CirclePolygon(position, self.r_out,
Expand Down Expand Up @@ -652,25 +699,16 @@ def get_fractions(self, data, method='exact', subpixels=5):
method=method, subpixels=subpixels)
return fractions

def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):


def plot(self, ax=None, fill=False, source_id=None, **kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill

if ax is None:
ax = plt.gca()

if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

theta_deg = self.theta * 180. / np.pi
for position in positions:
for position in plot_positions:
patch = mpatches.Ellipse(position, 2.*self.a, 2.*self.b,
theta_deg, **kwargs)
ax.add_patch(patch)
Expand Down Expand Up @@ -807,23 +845,16 @@ def __init__(self, positions, a_in, a_out, b_out, theta):
def area(self):
return math.pi * (self.a_out * self.b_out - self.a_in * self.b_in)

def plot(self, ax=None, fill=False, source_id=None, **kwargs):
def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill

if ax is None:
ax = plt.gca()

if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

theta_deg = self.theta * 180. / np.pi
for position in positions:
for position in plot_positions:
patch_inner = mpatches.Ellipse(position, 2.*self.a_in,
2.*self.b_in, theta_deg, **kwargs)
patch_outer = mpatches.Ellipse(position, 2.*self.a_out,
Expand Down Expand Up @@ -970,30 +1001,23 @@ def __init__(self, positions, w, h, theta):
def area(self):
return self.w * self.h

def plot(self, ax=None, fill=False, source_id=None, **kwargs):
def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill

if ax is None:
ax = plt.gca()

if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

hw = self.w / 2.
hh = self.h / 2.
sint = math.sin(self.theta)
cost = math.cos(self.theta)
dx = (hh * sint) - (hw * cost)
dy = -(hh * cost) - (hw * sint)
positions = positions + np.array([dx, dy])
plot_positions = plot_positions + np.array([dx, dy])
theta_deg = self.theta * 180. / np.pi
for position in positions:
for position in plot_positions:
patch = mpatches.Rectangle(position, self.w, self.h, theta_deg,
**kwargs)
ax.add_patch(patch)
Expand Down Expand Up @@ -1173,20 +1197,13 @@ def __init__(self, positions, w_in, w_out, h_out, theta):
def area(self):
return self.w_out * self.h_out - self.w_in * self.h_in

def plot(self, ax=None, fill=False, source_id=None, **kwargs):
def plot(self, origin=(0, 0), source_id=None, ax=None, fill=False,
**kwargs):

import matplotlib.pyplot as plt
import matplotlib.patches as mpatches

kwargs['fill'] = fill

if ax is None:
ax = plt.gca()

if source_id is None:
positions = self.positions
else:
positions = self.positions[np.atleast_1d(source_id)]
plot_positions, ax, kwargs = self._prepare_plot(
origin, source_id, ax, fill, **kwargs)

sint = math.sin(self.theta)
cost = math.cos(self.theta)
Expand All @@ -1196,12 +1213,12 @@ def plot(self, ax=None, fill=False, source_id=None, **kwargs):
hh_inner = self.h_in / 2.
dx_inner = (hh_inner * sint) - (hw_inner * cost)
dy_inner = -(hh_inner * cost) - (hw_inner * sint)
positions_inner = positions + np.array([dx_inner, dy_inner])
positions_inner = plot_positions + np.array([dx_inner, dy_inner])
hw_outer = self.w_out / 2.
hh_outer = self.h_out / 2.
dx_outer = (hh_outer * sint) - (hw_outer * cost)
dy_outer = -(hh_outer * cost) - (hw_outer * sint)
positions_outer = positions + np.array([dx_outer, dy_outer])
positions_outer = plot_positions + np.array([dx_outer, dy_outer])

for i, position_inner in enumerate(positions_inner):
patch_inner = mpatches.Rectangle(position_inner,
Expand Down