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

Use only is_circular argument in plot_dist #1681

Merged
merged 6 commits into from
May 5, 2021
Merged

Use only is_circular argument in plot_dist #1681

merged 6 commits into from
May 5, 2021

Conversation

agustinaarroyuelo
Copy link
Contributor

@agustinaarroyuelo agustinaarroyuelo commented Apr 28, 2021

Description

circular was used to indicate "taylor" bandwidht for the kde function and it co existed with the is_circular argument. In this PR I removed circular in favor of is_circular.

Checklist

  • Follows official PR format
  • New features are properly documented (with an example if appropriate)?
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

@@ -522,6 +522,8 @@ def kde(x, circular=False, **kwargs):
arviz.stats.density_utils.kde: Arviz KDE estimator
"""
if circular:
if circular == "degrees":
Copy link
Contributor Author

@agustinaarroyuelo agustinaarroyuelo Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that _kde_circular assumes inputs are in radians I added this internal conversion. We can discuss where else we can place it or if it should exist at all.

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I think the conversion is fine where it is

@agustinaarroyuelo agustinaarroyuelo changed the title [WIP] Use only is_circular argument in plot_dist Use only is_circular argument in plot_dist May 3, 2021
@OriolAbril OriolAbril merged commit bd0b786 into arviz-devs:main May 5, 2021
@agustinaarroyuelo agustinaarroyuelo deleted the circular_dist_plot branch May 6, 2021 12:27
utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
* use only is_circular argument

* change xlabels

* update posteriorplot and traceplot

* update bokeh plots

* add internal conversion for inputs in degrees

* update changelog and docstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants