Skip to content

Fix some lints - #1171

Merged
vincentarelbundock merged 2 commits into
vincentarelbundock:mainfrom
etiennebacher:lints
Jul 15, 2024
Merged

vincentarelbundock merged 2 commits into
vincentarelbundock:mainfrom
etiennebacher:lints

Conversation

@etiennebacher

Copy link
Copy Markdown
Contributor

Hi @vincentarelbundock, I've made a new package to find lints and automatically fix them (flint). I used marginaleffects as one of my test cases and found a few lints so I thought you might want to remove them. In particular, I think it might have fixed a bug in R/methods_nnet.R.

FYI those fixes were automatically generated with flint::fix_dir("R", exclude_linters = "equal_assignment"). End of the self-advertising :)

@vincentarelbundock

Copy link
Copy Markdown
Owner

Thanks @etiennebacher. The new tool looks great.

Yes, this is definitely a bug. Good catch.

Unfortunately, it also looks like flint introduced a new bug—or at least a departure from intended behavior.

model <- glm(mpg ~ hp, data = mtcars)

# same behavior
sup <- c("glm", "lm")
isTRUE(all(sup %in% class(model)))
> [1] TRUE
isTRUE(all(inherits(model, sup)))
> [1] TRUE

# different behavior
sup <- c("glm", "junk")
isTRUE(all(sup %in% class(model)))
> [1] FALSE
isTRUE(all(inherits(model, sup)))
> [1] TRUE

@etiennebacher

Copy link
Copy Markdown
Contributor Author

Thanks, nice catch, I reverted the change here and added a fix in flint

@vincentarelbundock
vincentarelbundock merged commit 2c8fc7e into vincentarelbundock:main Jul 15, 2024
@etiennebacher
etiennebacher deleted the lints branch July 15, 2024 13:42
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.

2 participants