Skip to content

Conversation

@wildart
Copy link
Contributor

@wildart wildart commented Apr 9, 2022

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Apr 9, 2022

Codecov Report

Merging #12 (6a8f80f) into main (6680786) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #12   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           36        36           
=========================================
  Hits            36        36           
Impacted Files Coverage Δ
src/regressionmodel.jl 100.00% <ø> (ø)
src/statisticalmodel.jl 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6680786...6a8f80f. Read the comment docs.

@nalimilan
Copy link
Member

Sorry for the delay. Do you think this is appropriate for RegressionModel rather than for StatisticalModel in general? I guess the reasoning is that only regression models have the concept of response? It's just kind of funny to consider e.g. PCA as a regression model, isn't it?

@wildart
Copy link
Contributor Author

wildart commented Apr 19, 2022

I don't know. Sure, it's a bit funny, but it fills like predict and reconstruct should go together. It would be awkward to have backward transformation in statistical model without forward one. What do you think of moving predict info StatisticalModel?

@nalimilan
Copy link
Member

I don't know. Sure, it's a bit funny, but it fills like predict and reconstruct should go together. It would be awkward to have backward transformation in statistical model without forward one. What do you think of moving predict info StatisticalModel?

Yeah, it seems better to add this function for RegressionModel than for StatisticalModel. Anyway, this is really a matter of convention since we don't provide any fallbacks.

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
@nalimilan nalimilan merged commit 540b43c into JuliaStats:main Apr 22, 2022
@nalimilan
Copy link
Member

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.

3 participants