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.

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

Allow any iterable object for the contours attribute #2786

Closed
sgdecker opened this issue Nov 10, 2022 · 2 comments
Closed

Allow any iterable object for the contours attribute #2786

sgdecker opened this issue Nov 10, 2022 · 2 comments
Labels
Area: Plots Pertains to producing plots Type: Feature New functionality

Comments

@sgdecker
Copy link
Contributor

What should we add?

I failed to think more generally the last time I submitted a similar issue (#2122). Code like the following generates the desired result (with a suitably defined div):

from itertools import chain

from metpy.plots.declarative import ContourPlot, MapPanel, PanelContainer

# Nonreproducible code skipped

def contours_pn(upper, interval=1):
    return chain(range(-upper, 0, interval), range(interval, upper+1, interval))

cp = ContourPlot()
cp.data = div
cp.linestyle = None
cp.contours = list(contours_pn(10))
cp.scale = 1e5
cp.clabels = True

mp = MapPanel()
mp.layout = (1,1,1)
mp.area = [-120, -65, 20, 55]
mp.layers = ['coastline', 'states', 'borders']
mp.plots = [cp]

pc = PanelContainer()
pc.size = (12, 10)
pc.panels = [mp]
pc.save('div.png')

div

However, omitting the list call, which seems superfluous, crashes:

---------------------------------------------------------------------------
TraitError                                Traceback (most recent call last)
Input In [29], in <cell line: 4>()
      2 cp.data = div
      3 cp.linestyle = None
----> 4 cp.contours = contours_pn(10)
      5 cp.scale = 1e5
      6 cp.clabels = True

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/metpy/plots/declarative.py:526, in ValidationMixin.__setattr__(self, name, value)
    524 allowlist.extend(self.trait_names())
    525 if name in allowlist or name.startswith('_'):
--> 526     super().__setattr__(name, value)
    527 else:
    528     closest = get_close_matches(name, allowlist, n=1)

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/traitlets/traitlets.py:715, in TraitType.__set__(self, obj, value)
    713     raise TraitError('The "%s" trait is read-only.' % self.name)
    714 else:
--> 715     self.set(obj, value)

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/traitlets/traitlets.py:689, in TraitType.set(self, obj, value)
    688 def set(self, obj, value):
--> 689     new_value = self._validate(obj, value)
    690     try:
    691         old_value = obj._trait_values[self.name]

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/traitlets/traitlets.py:721, in TraitType._validate(self, obj, value)
    719     return value
    720 if hasattr(self, "validate"):
--> 721     value = self.validate(obj, value)  # type:ignore[attr-defined]
    722 if obj._cross_validation_lock is False:
    723     value = self._cross_validate(obj, value)

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/traitlets/traitlets.py:2174, in Union.validate(self, obj, value)
   2172         except TraitError:
   2173             continue
-> 2174 self.error(obj, value)

File /usr/local/python/3.8/envs/met433/lib/python3.9/site-packages/traitlets/traitlets.py:827, in TraitType.error(self, obj, value, error, info)
    821 else:
    822     e = "The '{}' trait expected {}, not {}.".format(
    823         self.name,
    824         self.info(),
    825         describe("the", value),
    826     )
--> 827 raise TraitError(e)

TraitError: The 'contours' trait of a ContourPlot instance expected a list or an int or a range, not the chain at '0x7f7230b25be0'.

This is definitely in the weeds, easy to work around, and there are bigger fish to fry, but this would be a nice enhancement. On the other hand, this would allow potentially infinite iterable objects to be specified, although I have a hard time thinking of a case where that might happen.

Reference

No response

@sgdecker sgdecker added the Type: Feature New functionality label Nov 10, 2022
@dopplershift
Copy link
Member

dopplershift commented Nov 10, 2022

So while that would be nice, the problem is that matplotlib itself doesn't allow it:

from itertools import chain

import matplotlib.pyplot as plt
import numpy as np

def contours_pn(upper, interval=1):
    return chain(range(-upper, 0, interval), range(interval, upper+1, interval))

x = np.linspace(-3, 3)
y = np.linspace(-2, 2, 80)
vals = x**2 + y[:, None]**2
plt.contour(x, y, vals, contours_pn(10))

gives TypeError: float() argument must be a string or a real number, not 'itertools.chain' (but like above wrapping in list() fixes it).

So all MetPy could do here is relax the traitlet typing and call list anyway internally--and actually doing that correctly for the case of being given an int starts to make it complicated. All we do right now is directly pass it to matplotlib.

IMO the additional complexity, combined with the fact that we'd be diverging from matplotlib's API here makes me think it's not worth it. I'm willing to listen to arguments to the contrary, though.

@dopplershift dopplershift added the Area: Plots Pertains to producing plots label Nov 10, 2022
@sgdecker
Copy link
Contributor Author

Aw, crikey! If matplotlib doesn't accept more general objects, it makes sense to stay consistent on the MetPy side, so I'll go ahead and close this.

@dopplershift dopplershift closed this as not planned Won't fix, can't repro, duplicate, stale Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Plots Pertains to producing plots Type: Feature New functionality
Projects
None yet
Development

No branches or pull requests

2 participants