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

Update statisticalmodel.jl #27

Closed
wants to merge 1 commit into from
Closed

Update statisticalmodel.jl #27

wants to merge 1 commit into from

Conversation

likanzhan
Copy link

Two typos

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (20b38e1) 100.00% compared to head (249f2e2) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #27   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           37        37           
=========================================
  Hits            37        37           

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

@@ -170,11 +170,14 @@ Indicate whether the model has been fitted.
function isfitted end

"""
fit(model::StatisticalModel)
Copy link
Member

Choose a reason for hiding this comment

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

This method signature isn't quite right; packages typically define fit(::Type{M}, args...; kwargs...) where M is a StatisticalModel subtype defined in the package. However, I believe we avoided specifying an expected method signature in the docstring in order to leave it open-ended for packages that deviate for whatever reason. (Perhaps @nalimilan can correct me if I'm mistaken there.)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure there's a real intention behind the absence of the signature in the docstring. I would think the main reason is that there's no signature imposed, other than the first argument being a type as @ararslan said. Maybe documenting that would be useful at least? But we should check on JuliaHub that packages follow that minimal convention.

@likanzhan likanzhan closed this by deleting the head repository Feb 1, 2024
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.

4 participants