WIP: Add coords argument to plot_density #832
Conversation
rpgoldman
force-pushed
the
coords-arg-for-density
branch
3 times, most recently
from
06c526a
to
814c208
Compare
rpgoldman
force-pushed
the
coords-arg-for-density
branch
from
814c208
to
0f85a8d
Compare
| if coords is None: | ||
| coords = {} | ||
| datasets = [ | ||
| get_coords(convert_to_dataset(datum, group=group), coords) for datum in data |
There was a problem hiding this comment.
I think this will probably break if one of the inference data objects has a coordinate the other doesn't, which currently is one possibility in plot_density. Using inference data sel method instead of dataset sel will solve the issue with coordinate names by applying only the existing ones, but the case of mismatching coordinate values would not be solved.
As I do not think this will be a common case, maybe the best option is to create a helper function in densityplot.py itself in order to take this 2 possibilities into account?
There was a problem hiding this comment.
I'm not sure what to do about this. Maybe it would help to articulate what limits we put on the set of InferenceData objects we accept here? Without doing something like that, I don't see how we can say whether or not this function will plot correctly.
To be honest, I'm not sure what is gained by permitting a set of InferenceData here instead of requiring the user to invoke this function multiple times, once per InferenceData object. Presumably the issue is for model comparison, but I'm afraid I'm just guessing.
Because I don't know what is the answer to that, I don't know how to think about what we need to support here.
|
Checking in on old PRs. Any possibility of bringing this over the line or should we close? |
|
Closing this for now. If its still active and needs to be reopened feel free to do so! |
Not sure that this works properly yet. I'm kind of concerned about letting people specify coords to a function that accepts an Iterable of InferenceData, instead of just a single InferenceData.
Are we really sure we are confident in plotting multiple InferenceData? It seems like it might make the meaning of various parameters uncertain. Mightn't it be better to just have the caller loop over the data sets instead of having arviz do it autonomously?