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

Checks without error #9

Open
gaborcsardi opened this issue Oct 7, 2014 · 14 comments
Open

Checks without error #9

gaborcsardi opened this issue Oct 7, 2014 · 14 comments

Comments

@gaborcsardi
Copy link

Just an idea that came up as I was writing checks. It would be often useful to run the checks and get a TRUE/FALSE result instead of an error, similarly to see_if in assertthat.

Right now I need to duplicate the code for this, or use a single expression in ensure_that:

is_adjmatrix <- function(x) (is.matrix(x) || is(x, "Matrix")) && nrow(x) == ncol(x)
ensure_adjmatrix <- ensures_that(is_adjmatrix(.))

But then of course individual failed conditions are not reported.

Just an idea to think about.

@gaborcsardi gaborcsardi changed the title Checks without warning Checks without error Oct 7, 2014
@smbache
Copy link
Owner

smbache commented Apr 1, 2015

First, sorry, I seem to have forgotten about this.

How about a wrapper around ensures_that, say

check_that <- function(., ...)
{
  env  <- new.env(parent = parent.frame())
  env[["ensures_that"]] <- ensures_that

  args <- c(eval(substitute(alist(...))), fail_with = identity)
  cl <- as.call(c(quote(ensures_that), args))

  check_ <- eval(cl, env, env)

  !inherits(check_(.), "simpleError")
}

# short-hand version
check <- check_that

This is actually very close to how ensure_that is also defined.

Examples:

check_that(1:10, is.vector(., "numeric")) # => TRUE
check_that(1:10, is.vector(., "character")) # => FALSE

type_square <- ensures_that(identical(NCOL(.), NROW(.)))
check_that(iris, is.data.frame) # => TRUE
check_that(iris, is.data.frame, +type_square) # => FALSE

You can try it out in the classic branch.

@gaborcsardi
Copy link
Author

Hmmm. I have forgotten about this, too..... I think check_that looks pretty good.

I have actually started writing another package for argument checks, where the argument names and the assertions are together, at the same place in the code. (See https://github.com/gaborcsardi/argufy.) I have only noticed (= @jimhester told be about it) later that you have something very similar with function_. It is funny that we both used the tilde to separate the assertions. :)

@smbache
Copy link
Owner

smbache commented Aug 11, 2015

If you see value in it I'm happy to merge ideas.

@gaborcsardi
Copy link
Author

On thing I should probably do is to make use of ensure_ functions in my assertions. E.g. allow writing

inv <- argufy(function(
    M ~ ensure_square,
    algorithm = c("this", "that") ~ is_enum) {
  ...
})

So essentially ensurer can be an engine for argufy. Maybe similarly with assertthat.

I'll have to think about it a bit more. There are a bunch on design decisions to make. It would be great to build something people actually like using. I like ensurer, but people somehow did not pick it up (yet?). Why? Do you use it yourself? Similarly with assertthat, checkmate, etc.

Btw. the nice thing with argufy is that it is a build time dependency. I am not saying this is a major issue, but it is nice to have, anyway.

@smbache
Copy link
Owner

smbache commented Aug 11, 2015

I use it a lot. I think it has some use; I do see it around. But in general, I think making people check things is generally hard to popularize, as the benefits are not immediate, and it costs you some to write the extra lines/checks (you have to experience how much trouble you get in by not being careful first)... The appeal is therefore mostly on certain types of programmers and not the masses (like eg magrittr)

@gaborcsardi
Copy link
Author

Yes, I agree. We need to make checks very-very easy, otherwise people won't use it.
I think this includes mainly 1) a syntax that is only minimally more than what they would normally do, and 2) some pre-made assertions.

@smbache
Copy link
Owner

smbache commented Aug 11, 2015

How about having the ensuring (including the arguments) mechanism in ensurer (you can join in) and then make a package with some well designed "types". I like the idea about this kind of flexible types system...

@gaborcsardi
Copy link
Author

I don't mind merging in general. Two thoughts, though.

  1. In the spirit of making checks as easy for users as possible, I think we should include some assertions. in the package. In general, I like micro-packages but in this particular case I think a "batteries included" approach is better, to help people get started.

  2. In the same spirit, I think it would be great to add checks without having to define types. In argufy we worked out a syntax that requires minimal changes and efforts to add checks and coercions. It looks like this:

    f <- function(
      x =         ?! is.integer,
      y = "foo"   ?~ as_character,
      z 
    ) {
      y
    }

    The user needs to run argufy() on the functions. This can be done for all functions of package at once, with an argufy_package() call (to be merged from @jimhester). So essentially all users need to do is to add the ?! is.integer part to add a new check.

    Anyway, sorry for the long detour, my point is that while I don't mind having types, I think having the ability to add argument checks without defining types is important. It also fits into the spontaneous nature of the R language I think.

What do you think?

@smbache
Copy link
Owner

smbache commented Aug 11, 2015

From what I understand here, I like it.

Is there a benefit to have argufy rather than a special function constructor ala function_? Seems it's just an additional function call and less brief..

@gaborcsardi
Copy link
Author

No major differences, I guess. With Jim's PR, you can run argufy_package() in the end, so you don't have to add argufy() calls everywhere.

In general I feel more confident if the function is parsed as a function. This makes it less flexible of course, because I cannot have an argument declared as x ~ integer, while you can.

Other minor benefits:

  • You can use argufy() on existing functions, e.g. argufy(base::tolower, x = ~ is.character) (although this one does not happen to need it....)
  • If checks are expensive, and you don't want them in production, you just define ``? <- function(x, y) x (or include it from the package), and then they are not generated. (I am not sure if this binary approach is any good, though. Ideally you would want to disable some checks only, I guess.)
  • argufy is a build time dependency right now. The function_() approach could be as well, in theory, although right now it is not.

I am now leaning towards not merging. I think ensurer is great for pipes, and that it is good to have another package for argument checks. E.g. I want to keep the package as a build time dependency, and that is hard with ensurer checks. Not impossible, but it would require enough changes to make it worth to have another package.

@smbache
Copy link
Owner

smbache commented Aug 11, 2015

Ok. Fair enough. I'll have to look more into it - but I wouldn't mind two different pkgs; would be nice though if they would be designed with each other in mind though... And then stick to the more intuitive/clear way. Luckily the ensurer approach is only in the development version so no commitment to keeping it.

@egnha
Copy link

egnha commented Jan 26, 2017

@smbache, do you recall the discussion about purrr issue #275 concerning a complement to purrr::safely()strictly()—that would do post-factum argument checking? In meantime, I've implemented that functionality as a rather specialized package, valaddin (though without resorting to meta-programming, as I had originally proposed in #275, and instead taking inspiration from magrittr).

I only came across this thread today. Now I realize that the two of you (@gaborcsardi, @smbache) had already discussed and hashed out many of the ideas that I'd struggled with myself. It seems that the idea of using formulas to encode check expressions must be "canonical," for I came upon that idea, as well.

But in thinking about the challenge of functional input validation, I also came up with a small twist in perspective, which turned out to lead to a simpler and more flexible user interface than one that relies on a specialized function declaration: rather than grappling with arguments with type, I implemented predicates with scope. It could well be that this is the only novel part of valaddin, in view of what has already been discussed here. Nonetheless, this seemingly trivial change in perspective makes it possible to layer checks, and to employ a unified expression for them, which can handle type validations as easily as more general ones, such as those involving several arguments in combination. The README for valaddin gives an example of a set of checks (type checks, along with some linear constraints) that can be naturally applied in the context of predicates-with-scope but only awkwardly so in a context of argument-wise type validations.

Does valaddin address the main issues in this thread?

@smbache
Copy link
Owner

smbache commented Jan 26, 2017

Hm - not exactly sure what the question is, but maybe that's bc I'm not really sure what the main issue in this thread is, or whether it is still relevant ;)

@egnha
Copy link

egnha commented Jan 27, 2017

What I understood the "main issues" in this thread to be, is the problem of devising a simple mechanism for creating functions with validation, which is what valaddin solves in a functional-operator manner, in a way complementary to what has been discussed here (what I'd call a function declaration approach).

In any case, it seems what had been discussed in this old thread has in the meantime been implemented in the argufy and typeCheck packages. So maybe I am flogging a dead horse. ;)

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

3 participants