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

F() overriding FALSE #194

Closed
edrubin opened this issue Nov 4, 2021 · 3 comments
Closed

F() overriding FALSE #194

edrubin opened this issue Nov 4, 2021 · 3 comments

Comments

@edrubin
Copy link

edrubin commented Nov 4, 2021

First off: Thank you so much for a fantastic package.

I keep running into a problem generated by the F() function: it has overwritten base R's logical vector (for FALSE). Is there any way we give F() a different name? I know there's an issue of backward compatibility, but a lot of my previous code also breaks when collapse::F() masks base::F.

Simple example of the problem

library(data.table, collapse)
# Create a data table
> dt = data.table(var = c(TRUE, FALSE), val = 1:2)
# View data table
> dt
     var val
1:  TRUE   1
2: FALSE   2
# Filter where 'var' is FALSE (shorthand)
> dt[var == F]
Error: comparison (1) is possible only for atomic and list types
# Filter where 'var' is FALSE
> dt[var == FALSE]
     var val
1: FALSE   2

This behavior seems pretty undesirable (especially since T and F are so much quicker to type).

Thanks for considering my issue!

@edrubin edrubin changed the title F and FALSE F() overriding FALSE Nov 4, 2021
@SebKrantz
Copy link
Owner

Hi, thanks, this has come up before. I can think about depreciating F, but I actually wouldn't like that because it's indeed a backward compatibility issue. Th solution to your code is simply creating F <- FALSEin the global environment. In base R Fis simply a variable exported from the base package, just like collapse exports variables like .FAST_STAT_FUN.

@edrubin
Copy link
Author

edrubin commented Nov 4, 2021

Totally understand the backwards compatibility issue, but it seems like many more people use F as FALSE than as collapse::F(). Your solution would also work for backwards compatibility issues, e.g., F <- FF.

Either way, thanks for your work on a terrific package—and for your quick reply!

@SebKrantz
Copy link
Owner

Thanks, I agree with that. I wouldn't rename F to FFbut simply remove it. On the other hand I don't totally dislike the idea that using collapse would compell some peopel to write more secure code to avoid having to create an additional variable F. I'll keep thinking about it.

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

No branches or pull requests

2 participants