Skip to content

Conversation

@jazzido
Copy link
Contributor

@jazzido jazzido commented Feb 2, 2016

Hey @jeremybmerrill, @mtigas,

I've just integrated @mcharters's contribution to Tabula-UI. Can you review and help me test it?

Thanks!

@jeremybmerrill
Copy link
Member

What should I be looking for?

@jazzido
Copy link
Contributor Author

jazzido commented Feb 3, 2016

  • Obvious mistakes in the code (it's a 3-line change)
  • table detection: throw some PDFs at it and check how the autodetection feature performs.

@jazzido
Copy link
Contributor Author

jazzido commented Feb 17, 2016

Yo @jeremybmerrill, did you have the chance to check this?

@jeremybmerrill
Copy link
Member

nope, not yet! not sure when I"ll get a chance, but I put it on my calendar
for Sunday

On Wed, Feb 17, 2016 at 12:52 PM, Manuel Aristarán <notifications@github.com

wrote:

Yo @jeremybmerrill https://github.com/jeremybmerrill, did you have the
chance to check this?


Reply to this email directly or view it on GitHub
#456 (comment).

@jazzido
Copy link
Contributor Author

jazzido commented Feb 17, 2016

Cool. The patch is exactly 3 lines long, but I'd like that someone else throws a few tables at this thing.

@jeremybmerrill
Copy link
Member

looks good to me!

(I just ran some tables through, didn't seem to run itno issues)

@jeremybmerrill
Copy link
Member

and, I should say, autodetect worked in the cases that I tried

jazzido added a commit that referenced this pull request Feb 17, 2016
@jazzido jazzido merged commit 947cf31 into master Feb 17, 2016
@jazzido jazzido deleted the new-detection-algorithm branch February 17, 2016 21:17
jeremybmerrill pushed a commit that referenced this pull request Jun 26, 2018
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