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 fit docs #1746

Merged
merged 10 commits into from
May 17, 2024
Merged

Fix fit docs #1746

merged 10 commits into from
May 17, 2024

Conversation

ParadaCarleton
Copy link
Contributor

fit doesn't need a specific docstring just for Beta.

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.96%. Comparing base (818814f) to head (08ad0fb).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1746   +/-   ##
=======================================
  Coverage   85.96%   85.96%           
=======================================
  Files         144      144           
  Lines        8647     8647           
=======================================
  Hits         7433     7433           
  Misses       1214     1214           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ParadaCarleton
Copy link
Contributor Author

Any objections to merging this?

@jaksle
Copy link

jaksle commented Jul 11, 2023

I mentioned this in Issue #1701. For sure docstring for Beta should be removed, but currently fit has no general docstring for a distribution fit. The question is should it be added.

@devmotion
Copy link
Member

IMO we should add a docstring for generic fit, if this can be done in a clear and informative way, before removing the docstring for Beta. The possible problem with a generic docstring is that it is not clear which distributions are supported and what the (keyword) arguments are for the individual distributions.

@ParadaCarleton ParadaCarleton changed the title Fix Beta docs Fix fit docs Jul 13, 2023
@i9e1
Copy link

i9e1 commented Jul 14, 2023

maybe related #1748

I assume now that Poisson is not supported as stated in doc of fit_mle.

src/genericfit.jl Outdated Show resolved Hide resolved
src/genericfit.jl Outdated Show resolved Hide resolved
src/genericfit.jl Show resolved Hide resolved
ParadaCarleton and others added 2 commits July 14, 2023 16:23
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
@ParadaCarleton
Copy link
Contributor Author

ParadaCarleton commented Jul 14, 2023

Anything else to do before merging?

src/genericfit.jl Outdated Show resolved Hide resolved
@ParadaCarleton
Copy link
Contributor Author

Anything else or can I merge?

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

Is the docstring included in the docs?

src/genericfit.jl Outdated Show resolved Hide resolved
src/genericfit.jl Outdated Show resolved Hide resolved
ParadaCarleton and others added 2 commits July 18, 2023 12:44
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
@ParadaCarleton
Copy link
Contributor Author

Anything else?

@ParadaCarleton
Copy link
Contributor Author

@devmotion should I merge?

@ViralBShah
Copy link
Contributor

Merge?

@ParadaCarleton ParadaCarleton merged commit b09dcd7 into master May 17, 2024
12 of 15 checks passed
@ParadaCarleton ParadaCarleton deleted the beta-docs branch May 17, 2024 21:21
@devmotion
Copy link
Member

Why was this merged? Docs seem to be broken (missing docstrings): https://juliastats.org/Distributions.jl/dev/fit/

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

6 participants