Allow control of marker size (and other aspects of plt.scatter) #78
Conversation
|
@mikemhenry or @ijpulidos : Could you review this? |
cinnabar/plotting.py
Outdated
| statistic_type : str, default 'mle' | ||
| the type of statistic to use, either 'mle' (i.e. sample statistic) | ||
| or 'mean' (i.e. bootstrapped mean statistic) | ||
| marker_size : float, default 10 |
There was a problem hiding this comment.
I'm tempted to say that this point you really should be just passing in a dictionary of kwargs for matplotlib's scatter rather than incrementally adding new kwargs.
There was a problem hiding this comment.
Good point. I just pushed changes to address this suggestion. Let me know what you think
zhang-ivy
changed the title
cinnabar/plotting.py
Outdated
| bootstrap_x_uncertainty: bool = False, | ||
| bootstrap_y_uncertainty: bool = False, | ||
| statistic_type: str = "mle", | ||
| scatter_kwargs: dict = {}, |
There was a problem hiding this comment.
Would it be worth keeping s=10 and marker='o' as the defaults here? That way you have the same behaviour as before just with a little bit extra control?
IAlibay
left a comment
IAlibay
left a comment
There was a problem hiding this comment.
Just the one thing so I'll approve since it's just docs. @mikemhenry could you double check (sorry my brain is all mushy from this cold)?
cinnabar/plotting.py
Outdated
| statistic_type : str, default 'mle' | ||
| the type of statistic to use, either 'mle' (i.e. sample statistic) | ||
| or 'mean' (i.e. bootstrapped mean statistic) | ||
| scatter_kwargs : dict, default {} |
There was a problem hiding this comment.
Could you update the default value here too please?
There was a problem hiding this comment.
Yes, sorry about that. Done
3 participants
Description
Allow user to control the size of the markers (and other arguments) in plt.scatter()
Todos
Notable points that this PR has either accomplished or will accomplish.
scatter_kwargsargument toplotting._master_plotStatus