Skip to content

fit_annuli: plots=None renders all panels (can't disable via None) and leaks Matplotlib figures in loops #35

Description

@richteague

Summary

In rotationmap.fit_annuli, the plots argument defaults to None, but None is
interpreted as "plot all panels" rather than "plot nothing". To suppress plotting you
have to pass plots=[], which is neither documented nor obvious. Because the plotting
helpers open figures without closing them, calling fit_annuli in a loop (e.g. a
Monte-Carlo over many maps) leaks a figure per call — triggering matplotlib's
More than 20 figures have been opened warning and growing memory unbounded.

Version

eddy 3.1.0, eddy/rotationmap.py.

Where

# eddy/rotationmap.py, fit_annuli, ~L534
plots = ['profiles', 'model', 'residual'] if plots is None else plots
plots = np.atleast_1d(plots)
if 'profiles' in plots:
    self.plot_velocity_profiles(rpnts=rpnts, velo=velo, dvelo=dvelo)
if 'model' in plots:
    self.plot_model(model=model)
if 'residual' in plots:
    self.plot_model_residual(model=model)

Each of plot_velocity_profiles / plot_model / plot_model_residual does
fig, ax = plt.subplots() (e.g. rotationmap.py:2321) and never closes the figure.

Reproduce

import matplotlib.pyplot as plt
# rmap = some rotationmap / momentmap with a fittable map
for i in range(30):
    rmap.fit_annuli(x0=0, y0=0, inc=inc, PA=PA, rbins=rbins,
                    fit_vrad=True, fix_vlsr=vlsr, returns='residual')  # plots=None
print(len(plt.get_fignums()))   # ~90 open figures (3 per call); RuntimeWarning fires at 20

Expected vs. actual

  • Expected: plots=None (the default) produces no plots — the conventional meaning of
    an optional plots/plot kwarg — and repeated calls do not accumulate figures.
  • Actual: plots=None renders all three panels every call, and the figures are never
    closed, so a loop leaks figures and eventually warns / risks OOM.

Secondary: docstring mismatch

The docstring says:

plots (Optional[list]): Plots to generate after the fitting. Can be either of 'model'
and 'residual'. Default is both.

But the code default is three panels including 'profiles', and 'profiles' is a
valid (undocumented) option.

Suggested fix (your call)

  • Treat a falsy plots (None / [] / False) as no plots; if plot-by-default is
    wanted, use an explicit default list instead of the None sentinel.
  • Close the figures after drawing (or return them) so loop callers don't leak — or gate
    plotting behind an explicit plot=True.
  • Update the docstring to list 'profiles' and state the actual default.

Workaround

Pass plots=[] to suppress plotting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions