Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chain_prop docstring entry missing in matplotlib plot_trace() #1721

Open
hectormz opened this issue Jun 9, 2021 · 5 comments
Open

chain_prop docstring entry missing in matplotlib plot_trace() #1721

hectormz opened this issue Jun 9, 2021 · 5 comments

Comments

@hectormz
Copy link
Contributor

hectormz commented Jun 9, 2021

Describe the bug
chain_prop argument entry in matplotlib backend of plot_trace() docstring is missing.

It exists downstream at https://github.com/arviz-devs/arviz/blob/57aeded2fff88a646939fc36e5b2a86a0cf718b0/arviz/plots/traceplot.py and could be reproduced. But I'd like more information about valid properties that can be cycled. Can linestyle, color, lw, alpha, etc be used?

Also, somewhat related, I think that with the black formatting, this chunk at

else:
chain_prop = (
{
"linestyle": ("solid", "dotted", "dashed", "dashdot"),
}
if chain_prop is None
else chain_prop
)

would be clearer to read as:

        if chain_prop is None:
            chain_prop = {"linestyle": ("solid", "dotted", "dashed", "dashdot")}
@OriolAbril
Copy link
Member

I'm not sure backend files should have docstring with parameters at all 🤔 . Even backend specific things have to be documented on the main function so it would be purely duplicated content. It might come from the original backend split and it has drifted away from there as only the main file has been updated.

I like the style change.

@hectormz
Copy link
Contributor Author

hectormz commented Jun 9, 2021

By now (for me), I think it makes sense to have high-level docstrings only. I currently end up looking at the backends only to see how things are actually implemented, or to try to figure out what valid inputs are for certain parameters.

For heavily plot-specific parameters, the high-level docstrings might need to be more detailed. In the case of chain_props I'd like to know all the valid properties (and list of values) that will work. But those will differ for matplotlib and bokeh:

color vs line_color

linestyle vs line_dash etc

@OriolAbril
Copy link
Member

For heavily plot-specific parameters, the high-level docstrings might need to be more detailed. In the case of chain_props I'd like to know all the valid properties (and list of values) that will work. But those will differ for matplotlib and bokeh:

Yes, I think the best way to do that is to use interphinx and link to the function they are eventually passed to instead of trying to document this on our side, like we should do for kwargs too, see #1188

@hectormz
Copy link
Contributor Author

hectormz commented Jun 9, 2021

Right, basically anything on this matplotlib page is valid:

chain_prop={"linestyle": ("-","--"), "color":("red","green", "blue", "purple"), "linewidth":(1,4)},
chain_prop="color",
chain_prop={"dashes":((2,4),(1,5))}
chain_prop={"marker":["o","^"]}
chain_prop=None # defaults to pre-defined `linestyle` if `compact` is True

I realize that the high-level docstring should be updated to describe it as a dict instead of tuple:

    chain_prop: str or dict {str: array_like}, optional
        Tuple containing the property name and the property values to distinguish diferent chains

and maybe compact_prop needs to be updated, too

@hectormz
Copy link
Contributor Author

And the docstring examples need to be updated since compact=True is the default now.

Examples 2 and 5 (https://arviz-devs.github.io/arviz/api/generated/arviz.plot_trace.html) should be updated.

Example 2 should be (to illustrate what changing compact achieves):

az.plot_trace(data, compact=False)

and Example 5 should be

lines = [("theta_t", {'school': "Choate"}, [-1])]
az.plot_trace(data, var_names=('theta_t', 'theta'), coords=coords, lines=lines, compact=False)

or

lines = [("theta_t", {}, [-1])]
az.plot_trace(data, var_names=('theta_t', 'theta'), coords=coords, lines=lines)

and maybe consider whether the function should actually be changed to accommodate an 'over-specified' lines

lines = (('theta_t',{'school': "Choate"}, [-1]),)

az.plot_trace(data, var_names=('theta_t', 'theta'), coords=coords, lines=lines)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants