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

Add S3 group generics to .base_s3_generics, include exported S3 generics in generic list #1842

Merged
merged 11 commits into from
Dec 19, 2022

Conversation

AshesITR
Copy link
Collaborator

@AshesITR AshesITR commented Dec 13, 2022

fixes #1841 and #1808

@AshesITR AshesITR force-pushed the fix/1841-object_name-generics branch from 67bd22d to 79848fb Compare December 13, 2022 07:22
@AshesITR AshesITR changed the title Add S3 group generics to .base_s3_generics Add S3 group generics to .base_s3_generics, include exported S3 generics in generic list Dec 13, 2022
@AshesITR AshesITR requested review from IndrajeetPatil and MichaelChirico and removed request for IndrajeetPatil December 13, 2022 07:23
@@ -1,5 +1,5 @@
# Parse namespace files and return imports exports, methods
namespace_imports <- function(path = find_package()) {
namespace_imports <- function(path = find_package(".")) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous default was an error.

@@ -35,7 +35,7 @@ safe_get_exports <- function(ns) {
}

empty_namespace_data <- function() {
data.frame(pkg = character(), ns = character(), stringsAsFactors = FALSE)
data.frame(pkg = character(), fun = character(), stringsAsFactors = FALSE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All other code paths return a data frame with column names pkg and fun.

#' @export
#' Defined S3 generic in R/eat_me.R
#' Tests #1808
eat_me.liiiiiiiiiiiiiiiiiiiiiiiiiiist <- function(x, ...) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Class name has length 30, so this provokes object_length_linter().

Comment on lines +3 to +8
S3method("names<-",my_custom_class)
S3method(drink_me,data.frame)
S3method(drink_me,default)
S3method(drink_me,list)
S3method(eat_me,liiiiiiiiiiiiiiiiiiiiiiiiiiist)
S3method(head,my_s3_object)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are necessary for fixing #1808

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2022

Codecov Report

Merging #1842 (aa44914) into main (f4a7888) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head aa44914 differs from pull request most recent head 41ddeef. Consider uploading reports for the commit 41ddeef to get more accurate results

@@           Coverage Diff           @@
##             main    #1842   +/-   ##
=======================================
  Coverage   98.86%   98.86%           
=======================================
  Files         112      112           
  Lines        4825     4838   +13     
=======================================
+ Hits         4770     4783   +13     
  Misses         55       55           
Impacted Files Coverage Δ
R/namespace.R 100.00% <100.00%> (ø)
R/object_length_linter.R 100.00% <100.00%> (ø)
R/object_name_linter.R 98.18% <100.00%> (+0.06%) ⬆️

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

@IndrajeetPatil
Copy link
Collaborator

The builds still seem to be failing older R versions.

@AshesITR
Copy link
Collaborator Author

I'm looking into it, but it's slow for me to debug on old r versions.

@AshesITR
Copy link
Collaborator Author

🤦🏼‍♂️ the old stringsAsFactors = TRUE bit me.
image

R/namespace.R Outdated Show resolved Hide resolved
@IndrajeetPatil IndrajeetPatil added this to the 3.1.0 milestone Dec 19, 2022
@IndrajeetPatil IndrajeetPatil merged commit f9839b0 into main Dec 19, 2022
@IndrajeetPatil IndrajeetPatil deleted the fix/1841-object_name-generics branch December 19, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants