Skip to content

Add support for different docvars in structured text file - #147

Merged
kbenoit merged 11 commits into
quanteda:masterfrom
adamobeng:dev_textfile_columns
May 24, 2016
Merged

Add support for different docvars in structured text file#147
kbenoit merged 11 commits into
quanteda:masterfrom
adamobeng:dev_textfile_columns

Conversation

@adamobeng

Copy link
Copy Markdown
Contributor

Also changes the way corpora are combined to use data.table.

Fixes #131

adamobeng added 2 commits May 9, 2016 13:41
This removes the warning for column coercion

Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
@kbenoit

kbenoit commented May 9, 2016

Copy link
Copy Markdown
Collaborator

Not a bad idea, but it breaks the c.corpus method (why the build is failing - it's in the examples using c(corpus1, corpus2, ...)). It also changes the innards of the corpus object to a data.table, rather than a data.frame. We have to be very careful not to change the types of the objects or their internals through helper methods.

I suggest we think about moving to data.tables internally generally. We'd need to change the docvars.corpus() and [[.corpus methods though - and I suggest we talk it through before we start coding this.

@adamobeng

adamobeng commented May 10, 2016

Copy link
Copy Markdown
Contributor Author

My bad.

I'm setting up a git hook and writing some tests so that I don't do this again.

Probably not worth changing until we decide on re-writing textfile.R, though.

@kbenoit

kbenoit commented May 10, 2016

Copy link
Copy Markdown
Collaborator

In principle the way that textfile() handles the docvar structure can be separate from how they are stored in a corpus object - so could be considered separately from the textfile() rewrite.

adamobeng added 9 commits May 11, 2016 13:07
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
Signed-off-by: Adam Obeng <github@binaryeagle.com>
@codecov-io

Copy link
Copy Markdown

Current coverage is 7.66%

Merging #147 into master will increase coverage by 1.33%

@@            master      #147   diff @@
========================================
  Files           37        37          
  Lines         3383      3343    -40   
  Methods          0         0          
  Messages         0         0          
  Branches         0         0          
========================================
+ Hits           214       256    +42   
+ Misses        3169      3087    -82   
  Partials         0         0          

Powered by Codecov. Last updated by cc31d3c...8bed983

@kbenoit
kbenoit merged commit 700107b into quanteda:master May 24, 2016
@kbenoit

kbenoit commented May 24, 2016

Copy link
Copy Markdown
Collaborator

@adamobeng I accepted and merged, but you will want to check the functionality for the new line, to make sure it works. Also there are some tests failing in testTextfile now, please attend to these.

* checking tests ...
  Running ‘testthat.R’ ERROR
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
     })
  5: tryCatchList(expr, classes, parentenv, handlers)
  6: tryCatchOne(expr, names, parentenv, handlers[[1L]])
  7: value[[3L]](cont)
  8: stop("Data files do not have identical columns or variables")

  testthat results ================================================================
  OK: 110 SKIPPED: 0 FAILED: 2
  1. Failure: test structured textfile with glob-style mask (@testTextfile.R#139) 
  2. Error: test reading structured text files with different columns (@testTextfile.R#446) 

  Error: testthat unit tests failed
  Execution halted

* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... OK
* checking re-building of vignette outputs ... OK
* DONE
Status: 1 ERROR
checking tests ... ERROR
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
     })
  5: tryCatchList(expr, classes, parentenv, handlers)
  6: tryCatchOne(expr, names, parentenv, handlers[[1L]])
  7: value[[3L]](cont)
  8: stop("Data files do not have identical columns or variables")

  testthat results ================================================================
  OK: 110 SKIPPED: 0 FAILED: 2
  1. Failure: test structured textfile with glob-style mask (@testTextfile.R#139) 
  2. Error: test reading structured text files with different columns (@testTextfile.R#446) 

  Error: testthat unit tests failed
  Execution halted

@adamobeng

Copy link
Copy Markdown
Contributor Author

@kbenoit The tests in master pass on dev_textfile_columns. Why didn't you merge some of the changes to textfile.R?

@kbenoit

kbenoit commented May 24, 2016

Copy link
Copy Markdown
Collaborator

I thought I merged all of them... if they got dropped it's because I'm still learning the way to fetch, test/edit, and merge the pull requests.

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.

3 participants