tabulizer: Bindings for Tabula PDF Table Extractor Library

[leeper]

• 1. What does this package do? This page wraps the Tabula Java library, which can (very accurately) extract tables from PDF documents. It also implements some lower level utilities for working with PDF documents (metadata and text extraction, image conversion, split/merge). It should be useful for extracting scientific data, especially tabular data, from PDFs, such as from scientific articles or agency reports.
• 2. Paste the full DESCRIPTION file inside a code block below.

Package: tabulapdf
Type: Package
Title: Bindings for Tabula PDF Table Extractor Library
Version: 0.1.11
Date: 2016-05-07
Authors@R: c(person("Thomas J.", "Leeper", role = c("aut", "cre"),
                    email = "thosjleeper@gmail.com"))
Maintainer: Thomas J. Leeper <thosjleeper@gmail.com>
Description: Bindings for the Tabula <http://tabula.technology/> java library, which can extract tables from PDF documents.
License: MIT + file LICENSE
URL: https://github.com/leeper/tabulapdf
BugReports: https://github.com/leeper/tabulapdf/issues
Imports:
    graphics,
    grDevices,
    utils,
    tools,
    tabulizerjars,
    rJava,
    png
Suggests:
    testthat,
    knitr
RoxygenNote: 5.0.1

• 3. URL for the package (the development repository, not a stylized html page) https://github.com/leeper/tabulizer
• 4. What data source(s) does it work with (if applicable)? n/a
• 5. Who is the target audience? Data scientists stuck with other peoples' data
• 6. Are there other R packages that accomplish the same thing? If so, what is different about yours? The closest thing will be pdftools, which is a libpoppler wrapper. tabulizer has some overlap but the core functionality - table extraction - is not supported by pdftools.
• 7. Check the box next to each policy below, confirming that you agree. These are mandatory.
  • This package does not violate the Terms of Service of any service it interacts with.
  • The repository has continuous integration with Travis CI and/or another service
  • The package contains a vignette
  • The package contains a reasonably complete README with devtools install instructions
  • The package contains unit tests
  • The package only exports functions to the NAMESPACE that are intended for end users
• 8. Do you agree to follow the rOpenSci packaging guidelines? These aren't mandatory, but we strongly suggest you follow them. If you disagree with anything, please explain.
  • [] Are there any package dependencies not on CRAN? The package has some files in a dependent package, which can be put on CRAN once everything is ready to release.
  • Do you intend for this package to go on CRAN?
  • Does the package have a CRAN accepted license?
  • Did devtools::check() produce any errors or warnings? If so paste them below.
• 9. Please add explanations below for any exceptions to the above:
• 10. If this is a resubmission following rejection, please explain the change in circumstances.

[sckott]

Thanks for your submission @leeper Seeking reviewers now

[sckott]

Reviewers: @lmullen, @davidgohel
Due: 2016-06-01

[davidgohel]

This package is a very useful new package. It is also well written ; code is simple and clear, standards are respected. It was easy and fun to dive into the code. (Also I realized I could simplify some of my old
codes while reading yours!). 👏

This work is a very good reference for package developpement with rJava.

I have tested it on:

• mac OS (R version 3.2.4)
• Windows (R version 3.2.3 32 bits) - locale is french Cp1252

Everything worked as expected. Installation is simple and the vignette is clear. I enjoy the data.frame method, it worked well on relevant documents.

There is nothing to complain about in the code. However, I made efforts to find things to say ;)

---

Headless Mode

I found out one issue that can be easily solved. Headless Mode: it should be set to true by default (this can be an issue with RStudio server for example). If not, pdfbox might not work on some servers. See this article.

In .onLoad function, this can easily setup with

J('java.lang.System')$setProperty('java.awt.headless', 'true')

Encoding issue with Windows

I met one annoying issue. I made the french test, trying to use tabulizer with a document full of terrible accents. Everything went fine on mac, but I met encoding issues with Windows.

Note : I tried to set encoding parameters but it did not solve the issue.

J('java.lang.System')$setProperty('file.encoding', 'UTF-8')

It seems the problem come from pdfbox and the way encodings are managed (not sure about that).

To demo the issue:

1. Save manually the document from http://www.sfds.asso.fr/ressource.php?fct=ddoc&i=2090 to ZOOM_METIERS_MATHS_WEB_144dpi3.pdf in your working directory.
2. Run

f <- "ZOOM_METIERS_MATHS_WEB_144dpi3.pdf"
extract_text(f, pages = 2)

Below a partial copy of the result :

[1] "ALAURÉAT\r\nANNÉE\r\n1\r\nANNÉE\r\n2\r\nANNÉE\r\n6\r\n3e année\r\nANNÉE\r\n8\r\nANNÉE\r\n5\r\nÉCOLE\r\nD'INGÉNIEURS\r\n2e année\r\nANNÉE\r\n7\r\nÉCOLE\r\nD'INGÉNIEURS\r\nANNÉE\r\n3\r\nANNÉE\r\n4\r\nLICENCE PRO\r\nÉCOLE\r\nD'INGÉNIEURS\r\nClasse prépa2e année\r\nClasse prépa1re année\r\nCycle \r\nintégré\r\n1re année\r\nCycle \r\nintégré\r\n2e année\r\nL3\r\nL2\r\nL1\r\n1re année\r\nM2\r\nM1\r\nLICENCE\r\nMASTER\r\nDOCTORAT\r\nDIPLÔME\r\nD'ING

Below a partial copy of the result on my mac :

[1] "ALAURÉAT\nANNÉE\n1\nANNÉE\n2\nANNÉE\n6\n3e année\nANNÉE\n8\nANNÉE\n5\nÉCOLE\nD'INGÉNIEURS\n2e année\nANNÉE\n7\nÉCOLE\nD'INGÉNIEURS\nANNÉE\n3\nANNÉE\n4\nLICENCE PRO\nÉCOLE\nD'INGÉNIEURS\nClasse prépa2e année\nClasse prépa1re année\nCycle \nintégré\n1re année\nCycle \nintégré\n2e année\nL3\nL2\nL1\n1re année\nM2\nM1\nLICENCE\nMASTER\nDOCTORAT\nDIPLÔME\nD'ING

Minor issues / suggestions

DESCRIPTION file

I think R-Core team could ask you to add authors of the java packages as contributor and copyright holder (that was my case for package ReporteRsjars) in package tabulizerjars. To prevent this issue when submitting to CRAN, I would add Manuel Aristarán in field Authors@R of the DESCRIPTION file as:

person("Manuel", "Aristarán", role = c("ctb", "cph"), comment = "tabula java library")

file path with ~

It seems file path containing ~ are not being expanded (occurred on mac os)

Verbose logging

As a user, I would prefer not having the logger messages. If it can help, it can be switch off :

Solution 1 : Desable ALL loggers

J("java.util.logging.LogManager")$getLogManager()$reset()

Solution 2 : Desable only pdfbox loggers

BUT it would require org.apache.log4j in the classpath.

logManager <- rJava::J("java.util.logging.LogManager")$getLogManager()

loggers <- c("org.apache.pdfbox.util.PDFStreamEngine",
  "org.apache.pdfbox.pdmodel.font.PDSimpleFont",
  "org.apache.pdfbox.pdmodel.font.PDFont",
  "org.apache.pdfbox.pdmodel.font.FontManager",
  "org.apache.pdfbox.pdfparser.PDFObjectStreamParser")

for( logger_id in loggers ){
  logger <- logManager$getLogger(logger_id)
  logger$setLevel(J("org.apache.log4j.Level.OFF")) 
}

[leeper]

Thank you, @davidgohel! I have now implemented most of this, but am still working on the non-latin character issue and finalizing the logging functionality.

[sckott]

@lmullen @davidgohel
Due: 2016-06-01 - hey there, it's been 21 days, please get your review in soon, thanks 😺 (ropensci-bot)

[leeper]

Following up on the first review, I believe I have now addressed all issues:

1. rJava is configured to run in headless mode on package load.
2. I have added an encoding argument to extract_text() and extract_tables() that allows the specification of text encodings. I'm not 100% confident in this, but it seems to work.
3. I've updated authorship in DESCRIPTION of tabulizerjars to reflect the java library's authorship. I've also added @davidgohel's contributions as a reviewer.
4. I use path.expand() for local file paths.
5. I have added a stop_logging() function to turn off all Java logging. This seemed easier than adding a log4j dependency, but may not be ideal, so logging is left on by default.

Thank you again for the feedback!

[lmullen]

This is a very well done package. The code is a pleasure to read, and the interface is well thought out. The core functionality works as expected.

This package is so useful that I would really love it if this package worked on PDFs of 19th-century documents: alas, it's not to be. (Hardly the fault of the package, of course.)

@davidgohel has already offered a number of suggestions which have been dealt with, so I'll deal mostly with unexpected errors when using the interface.

Tests

I've run R CMD check on this package with R 3.3.0 on Mac OS X 10.11.5 and R 3.3.0 on Ubuntu 16.04. (The rest of my review was on the Mac. I don't have a Windows machine to test that installation, but the instructions in the README seemed clear.)

• The Travis integration on the master branch is failing.
• I get two test failures when building and running R CMD check (on both Mac and Ubuntu):

  OK: 73 SKIPPED: 0 FAILED: 2
  1. Failure: Read French language PDF w/correct encoding (@test_non-latin.R#17) 
  2. Failure: Read French language PDF w/correct encoding (@test_non-latin.R#18) 

I gather that this is part of the issue which @davidgohel mentioned which is being worked out.

Interface

For testing I used a PDF form the U.S. Census's Statistical Abstract

Some minor questions:

• Is there are reason why get_n_pages() takes an option doc = parameter first instead of the file = parameter, like most other functions in this package?
• Can split_pdf() take an argument like outdir = to specify where the split files should go? Also, it would be nice if the numbers in the new file names were zero padded. More importantly, split_pdf() overwrites files in the directory without warning. The function needs some way of requiring the user's permission before overwriting files, especially since the function doesn't let the user specify the file name or path.
• When using merge_pdfs() I get a warning message:

Warning message:
In if (grepl("^http.*://", path)) { :
  the condition has length > 1 and only the first element will be used

This comes from tabulizer:::localize_file(). The if() condition there probably needs to be if(any(CONDITION)). I suppose a user could pass the function a mix of remote and local PDFs so that the any() function would be insufficient, but that seems far-fetched.

• I get an error with a traceback when using the option spreadsheet = TRUE to extract_tables().

 Error in .jcall("RJavaTools", "Ljava/lang/Object;", "invokeMethod", cl,  : 
  java.lang.IndexOutOfBoundsException: Index: 0, Size: 0 

If this isn't a problem with the underlying Java code, then this error message should let the user know why the spreadsheet option shouldn't be used.

• Same comments for make_thumbnails() as for split_pdf() above.

After playing with it for a while, I was able to figure out the interface to extract_areas(). Would it be possible to make two improvements to that user interface? First, instead of clicking and the upper left and lower right of the area, can the user draw a bounding box? Second, can the bounding box be adjusted before the function returns?

Miscellaneous

• The coding style and documentation are very well done.
• I found it a little odd that the README had the installation instructions near the bottom, since it goes against the convention. Can you move the installation instructions up (maybe leaving the Windows Java stuff at the bottom)? See the README section of the packaging guide.
• The package has clear contributing guidelines. It would be good to also have the code of conduct as mentioned in the rOpenSci policies.
• The package has a good NEWS file. But it also needs to use tags for all the point releases. I don't see any point in going back to tag the previous versions, but it should be tagged starting with acceptance at rOpenSci.
• My two cents: I'd much prefer if the Java logging was off by default. I don't think it tells most users anything that they can actually use.

Nice work, and I look forward to using the package myself.

[leeper]

Thank you, @lmullen! I will work to address these points and post an update once I have finished.

[leeper]

I have attempted to address all of your comments and have made the following revisions:

1. I have stopped the failing encoding tests (but have reopened an issue to get this to work better cross-platform).
2. I have reordered the file and doc arguments to get_n_pages() for consistency and given get_page_dims() a doc argument, as well. I figure these may be useful for debugging.
3. split_pdf() and make_thumbnails() gain an outdir argument. In both cases, output files are numbered using zero padding, as requested.
4. I have fixed the merge_pdfs() warnings by avoiding passing a vector of filenames through localize_file().
5. The error related to the spreadsheet = TRUE argument has been corrected and was quite critical, actually, as it related to the attempt to coerce an empty Java array to an R matrix.
6. I have turned Java logging off by default and added some notes to the documentation to describe how to restore logging if that were, for some reason, preferred.
7. Regarding the behavior of extract_areas(), I agree this is less than ideal but the current implementation isn't actually as strict as the documentation implies (you can tag upper-left/lower-right or upper-right/lower-left). I have an open issue on improving this, but I think there are some fundamental limitations in locator() and graphics devices more generally for creating an intuitive interface since it is not really possible to dynamically highlight/select an area of a plot (as far as I know). I am open to suggestions on this.

On smaller points:

1. Regarding installation instructions, I think we're getting to the point where GitHub-based installation is sufficiently common that it doesn't need to be at the top of the README (since that seems to be a legacy of earlier times). I would actually suggest the general style guide be changed, accordingly. But I don't feel strongly about this, and can update if preferred.
2. I read the code of conduct provision as only a recommendation, so I have not added this.
3. My release workflow is to only tag "minor" not "patch" versions as GitHub releases and generally to only release to CRAN when minor ticks up, but I can adapt this to a different style if required.

Thank you, again, @davidgohel and @lmullen, for your feedback! It is very much appreciated.