Skip to content

Conversation

@MichaelChirico
Copy link
Member

@MichaelChirico MichaelChirico commented Aug 18, 2019

Closes #2109
Just as simple as Arun said 2 years ago 😅

@codecov
Copy link

codecov bot commented Aug 18, 2019

Codecov Report

Merging #3775 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3775   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files          71       71           
  Lines       13225    13225           
=======================================
  Hits        13148    13148           
  Misses         77       77
Impacted Files Coverage Δ
R/data.table.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2dbd3a0...400d0c9. Read the comment docs.

if (is.call(jsub) && deparse(jsub[[1L]], 500L, backtick=FALSE) %chin% c("!", "-")) { # TODO is deparse avoidable here?
# 2109 we should only match unary -
if (is.call(jsub) && deparse(jsub[[1L]], 500L, backtick=FALSE) %chin% c("!", "-") && length(jsub) == 2L) { # TODO is deparse avoidable here?
notj = TRUE
Copy link
Member

Choose a reason for hiding this comment

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

deparse(jsub[[1]]) == as.character(jsub)[[1]] afaik

Copy link
Member

@mattdowle mattdowle Aug 28, 2019

Choose a reason for hiding this comment

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

@HughParsonage Yep - that works. Now I'm puzzled why the deparse was there to start with. Ah well, tests pass.

@mattdowle mattdowle changed the title Closes #2109 -- restrict negation in !with to unary -/! restrict negation in !with to unary -/! Aug 28, 2019
@mattdowle mattdowle added this to the 1.12.4 milestone Aug 28, 2019
@mattdowle mattdowle merged commit b6f1dfe into master Aug 28, 2019
@mattdowle mattdowle deleted the minus_with branch August 28, 2019 02:33
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.

Wrong behaviour DT[, i, with=FALSE]

4 participants