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

fix bug in trendline in the case of missing values #2357

Merged
merged 8 commits into from Apr 27, 2020
Merged

Conversation

emmanuelle
Copy link
Contributor

See https://community.plotly.com/t/plotly-trendlines-not-displaying-correctly-due-to-np-nan-values/37202

The case of nan values was not handled properly for trendlines:

  • for lowess, the default value of statsmodels is to drop nans which is fine, but then the x values were not properly aligned with the y
  • for ols, the default value is to do nothing with nans, resulting in all nans in the estimation.

This PR fixes the two cases and adds a test.

@emmanuelle
Copy link
Contributor Author

@nicolaskruchten I think this PR is ready

@nicolaskruchten nicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
modes = ["ols", "lowess"]
for mode in modes:
fig = px.scatter(df, x="year", y="pop", color="country", trendline=mode)
country_numbers = len(fig["data"]) // 2
Copy link
Member

Choose a reason for hiding this comment

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

what does this line do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing :-). So I removed it. Thanks!

@nicolaskruchten
Copy link
Member

💃

@emmanuelle emmanuelle merged commit e778c6b into master Apr 27, 2020
@nicolaskruchten nicolaskruchten deleted the trendline-bug branch June 19, 2020 16:17
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.

None yet

2 participants