Skip to content

Make dfm_match() work with padding#1960

Merged
koheiw merged 1 commit into
masterfrom
hotfix-dfm_match
May 23, 2020
Merged

Make dfm_match() work with padding#1960
koheiw merged 1 commit into
masterfrom
hotfix-dfm_match

Conversation

@koheiw

@koheiw koheiw commented May 23, 2020

Copy link
Copy Markdown
Collaborator

In koheiw/newsmap#39, I noticed that dfm_match() crashes when features contain "".

toks <- tokens("a b !", padding = TRUE, remove_punct = TRUE)
dfmt <- dfm(toks)
dfm_match(dfm("a b c"), "") # error

The root cause is actually [.dfm

dfmt[,""] #  error
dfm_select(dfmt, "") # works

This is a deeper issue that requires careful redesigning of how we handle paddings, because R treats empty names as a special case according to ?names():

The name "" is special: it is used to indicate that there is no name associated with an element of a (atomic or generic) vector. Subscripting by "" will match nothing (not even elements which have no name).

@codecov

codecov Bot commented May 23, 2020

Copy link
Copy Markdown

Codecov Report

Merging #1960 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1960   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files         101      101           
  Lines        6388     6388           
=======================================
  Hits         5830     5830           
  Misses        558      558           

@koheiw
koheiw merged commit fe2c54f into master May 23, 2020
@koheiw
koheiw deleted the hotfix-dfm_match branch May 23, 2020 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants