performance icon indicating copy to clipboard operation
performance copied to clipboard

Export S3 methods for model_performance generic #452

Open jimrothstein opened this issue 2 years ago • 9 comments

jimrothstein avatar Oct 21 '22 05:10 jimrothstein

@jimrothstein Thanks a lot for doing this! I have modified it a bit to also update NAMESPACE.

@strengejacke I had to move the default method to where generic is defined because otherwise it won't be found by other methods that rely on the default method. The other solution would have been to use Collate field in DESCRIPTION, but I am not a fan of that option.

IndrajeetPatil avatar Oct 21 '22 05:10 IndrajeetPatil

Codecov Report

Merging #498 (551580e) into main (ba99475) will increase coverage by 0.31%. The diff coverage is 13.04%.

@@            Coverage Diff             @@
##             main     #498      +/-   ##
==========================================
+ Coverage   36.06%   36.37%   +0.31%     
==========================================
  Files          83       82       -1     
  Lines        5341     5341              
==========================================
+ Hits         1926     1943      +17     
+ Misses       3415     3398      -17     
Impacted Files Coverage Δ
R/model_performance.R 12.90% <13.04%> (+0.40%) :arrow_up:
R/model_performance.lm.R 73.95% <0.00%> (+1.04%) :arrow_up:
R/r2.R 37.45% <0.00%> (+5.99%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Oct 21 '22 05:10 codecov-commenter

@strengejacke yes, I should have checked where the .default method was defined.

jimrothstein avatar Oct 21 '22 05:10 jimrothstein

@jimrothstein Do you mind also updating the NEWS.md file and mention these changes?

Remember to include the PR number and your GitHub handle in the news item.

IndrajeetPatil avatar Oct 21 '22 06:10 IndrajeetPatil

@IndrajeetPatil, Yes, will do, but tomorrow ... (was trying to find away to generate all the @export for the methods via embedded code using knitr ... did not work.)

jimrothstein avatar Oct 21 '22 08:10 jimrothstein

@strengejacke I had to move the default method to where generic is defined because otherwise it won't be found by other methods that rely on the default method. The other solution would have been to use Collate field in DESCRIPTION, but I am not a fan of that option.

Sounds good to me!

strengejacke avatar Oct 23 '22 21:10 strengejacke

@jimrothstein Why did you close this?

@strengejacke I think this can be reopened and merged?

IndrajeetPatil avatar Jan 17 '23 08:01 IndrajeetPatil

@IndrajeetPatil @strengejacke

Hadn't heard from anyone in long time; and was cleaning repos.

Did not realize affect here, which is my fault, but I did not know.

jimrothstein avatar Jan 17 '23 09:01 jimrothstein

@IndrajeetPatil @strengejacke

Do you want me to continue with tinytest:: checks ?

jimrothstein avatar Jan 21 '23 19:01 jimrothstein

Closing this for now, for two reasons:

  1. https://github.com/easystats/parameters/pull/807#issuecomment-1494687759
  2. Not sure how to resolve conflicts with the command line.

strengejacke avatar Jul 13 '24 21:07 strengejacke