Improve line removal in model piping #879
Conversation
billdenney
left a comment
billdenney
left a comment
There was a problem hiding this comment.
@mattfidler Can you please take a look at these two tests? I think that we may need to change the behavior, but I'm not sure.
| ) | ||
| # `useErrorLine = TRUE` and `returnAllLines = FALSE` | ||
| # no lines with `a` on the LHS are returned | ||
| ## TODO: Is this the intended behavior? I expected line 1 to be returned. |
There was a problem hiding this comment.
@mattfidler, Can you please take a look at this? I think it may be a bug.
There was a problem hiding this comment.
I don't think it is a bug, I think this is used in modelExtract(), but I could be wrong.
There was a problem hiding this comment.
Perhaps I don't understand what you are trying to do enough to say.
There was a problem hiding this comment.
What I'm trying to do immediately is have simpler tests to understand the methods used. Then, with simpler tests, I can make modifications knowing that I won't accidentally break anything (and I don't have to wait for the full test suite to run).
So, overall right now, I'm adding tests for code behavior and logically checking that there aren't any accidental bugs. (I did find one where model(a ~ NULL) would not remove the line.) It sounds like this one is intended as-is.
|
|
||
| # `useErrorLine = TRUE` and `returnAllLines = FALSE`; altExpr gives the actual value | ||
| # `d` is never an LHS value so NULL is returned | ||
| # TODO: It's unclear why the warning "with single endpoint model prediction 'a' is changed to 'd'" occurs. |
There was a problem hiding this comment.
@mattfidler, Can you please take a look at this? The output makes sense to me, but the warning suggests that the model is changed when it is not. I'm not sure what the intent of the warning is.
No description provided.