Skip to content

Revert "use J as a binary instead of loading the module ( 120 + MBs )"#89

Merged
dbashford merged 1 commit intodbashford:masterfrom
tommygnr:use-j-as-module
Jun 27, 2016
Merged

Revert "use J as a binary instead of loading the module ( 120 + MBs )"#89
dbashford merged 1 commit intodbashford:masterfrom
tommygnr:use-j-as-module

Conversation

@tommygnr
Copy link
Contributor

@tommygnr tommygnr commented Jun 14, 2016

This reverts commit 6e35e0a made in #22

As I mentioned in a comment on that pull request the way npm v3 flattens dependencies means we can no longer rely on the j binary being in a consistent location relative to this package. I also think executing J in a separate process is the wrong way to go.

This change necessarily breaks #37 as well however without it textract can't be used as a dependency within another project to extract text from spreadsheets

@dbashford
Copy link
Owner

I'd rather at least check to see if it can be found either at the project root or where it's looking now before falling back to loading it in.

And if executing things in another process was wrong, textract has bigger issues :)

@dbashford
Copy link
Owner

FYI, just started a new job this week so may be until the weekend before I get back to it.

@tommygnr
Copy link
Contributor Author

And if executing things in another process was wrong, textract has bigger issues :)

Heh. I was only referring to running j in another process as it is a javascript library. Other binaries are a different story :-)

Other things I would say to support this change are:

  • memory consumption of j has been drastically reduced since this change was made 2 years ago
  • running j in a separate process is much slower and error prone

@dbashford
Copy link
Owner

Played with this a little, happy to go back.

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.

2 participants