-
Notifications
You must be signed in to change notification settings - Fork 5
Fix plotting with custom types as keys #502
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
Conversation
| else: | ||
| to_nodes = [(getattr(obj, "name", None), obj)] | ||
| nodes = [Node(processor, inp, name=name) for name, inp in to_nodes] | ||
| nodes = [Node(processor, inp, name=str(name)) for name, inp in to_nodes] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually the correct place for the string conversion, or would it be better to accept, e.g., any Hashable as name and add string-formatting where actually needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some more digging and I think this is the right place for the conversion.
It is not about string formatting, but how we handle different input types to the Node constructor.
The error that was being raised before the fix is
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In[3], line 8
5 A = NewType("A", str)
6 B = NewType("B", int)
----> 8 pp.plot({A: a, B: b})
File [~/code/path/plopp/plotting/plot.py:173](http://localhost:8965/home/nvaytet/code/path/plopp/plotting/plot.py#line=172), in plot(obj, aspect, autoscale, cbar, clabel, cmap, cmax, cmin, coords, errorbars, figsize, grid, ignore_size, legend, logc, logx, logy, mask_cmap, mask_color, nan_color, norm, scale, title, vmax, vmin, xlabel, xmax, xmin, ylabel, ymax, ymin, **kwargs)
171 ndims = set()
172 for n in nodes:
--> 173 ndims.add(n().ndim)
174 if len(ndims) > 1:
175 raise ValueError(
176 'All items given to the plot function must have the same '
177 f'number of dimensions. Found dimensions {ndims}.'
178 )
File [~/code/path/plopp/core/node_class.py:89](http://localhost:8965/home/nvaytet/code/path/plopp/core/node_class.py#line=88), in Node.__call__(self)
88 def __call__(self) -> Any:
---> 89 return self.request_data()
File [~/code/path/plopp/core/node_class.py:147](http://localhost:8965/home/nvaytet/code/path/plopp/core/node_class.py#line=146), in Node.request_data(self)
144 if self._data is None:
145 args = (parent.request_data() for parent in self.parents)
146 kwargs = {
--> 147 key: parent.request_data() for key, parent in self.kwparents.items()
148 }
149 self._data = self.func(*args, **kwargs)
150 return self._data
File [~/code/path/plopp/core/node_class.py:149](http://localhost:8965/home/nvaytet/code/path/plopp/core/node_class.py#line=148), in Node.request_data(self)
145 args = (parent.request_data() for parent in self.parents)
146 kwargs = {
147 key: parent.request_data() for key, parent in self.kwparents.items()
148 }
--> 149 self._data = self.func(*args, **kwargs)
150 return self._data
TypeError: _typing._idfunc() takes exactly one argument (0 given)
This happens because when we take in a dict of objects, we send each object through a pre-processor node which converts the input to a data array. One of the args of that pre-processor is the name of the data array (which is used for legends or axes labels).
The name gets wrapped into a Node itself because it is an input to the pre-processor node. When wrapping the name, the Node constructor checks if the thing it is wrapping is callable. The types A and B are callable and so they get handled as if they were a function that returns data, when in fact they should either be converted to a string, or just wrapped in a lambda function.
The only reason why it worked before with pp.plot({int: a, float: b}) and not with pp.plot({A: a, B: b}) is that int() yields 0 and float() yields 0.0 (i.e. it is possible to call int and float with no arguments, while calling A() and B() fails).
|
Is the example in the PR description correct? I don't understand what the figure should display. |
Yes! (updated) |
This used to raise with a strange error:
This is a common use case when computing multiple results from a sciline workflow, as it returns a dict with custom types as keys.