New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding .msg support #17

Closed
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@anthonygarvan
Contributor

anthonygarvan commented Jul 30, 2014

Adding microsoft outlook .msg support, based on Matt Walker's msg-extractor. Also fixed added a contributor guidelines section to the docs and properly linked to it (was copied over from flo previously).

@anthonygarvan anthonygarvan changed the title from Fixing doc reference, draft of CONTRIBUTING.md to Adding .msg support Jul 31, 2014

@anthonygarvan anthonygarvan referenced this pull request Jul 31, 2014

Closed

.msg support #5

# Copyright 2013 Matthew Walker
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by

This comment has been minimized.

@anseljh

anseljh Aug 4, 2014

Isn't it problematic to add GPL code to a repo that carries another kind of license?

@anthonygarvan

This comment has been minimized.

Contributor

anthonygarvan commented Aug 4, 2014

In general yes, but Wikipedia says the MIT license is GPL compatible. http://en.m.wikipedia.org/wiki/MIT_License. That's as far as I'll go, if we want 100% clarity we would need to talk to a copyright lawyer.

@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Aug 6, 2014

@anthonygarvan I really appreciate all the work you've put into this. I'm going to cherry pick the CONTRIBUTING.md contribution into master in a minute, but I tend to agree with @anseljh though and would prefer not to reproduce GPL libraries (or anyone else's code, for that matter) into our project. If not for the legal reason then for the functional reason that, if msg-extractor finds a new and better way to do things, then we'll have this awkward challenge of trying to keep our version up to date with the version that specializes in doing this.

I do, however, have a suggestion that I would be happy to pair-program with you to implement. This is a little more involved, but will ultimately help this project and the msg-extractor project out considerably.

The problem (that you found a great workaround for, by the way) is that msg-extractor does not have a setup.py. If it at least had a setup.py (and was preferrably on pypi) then it would be possible to just add msg-extractor to the list of python requirements and we could just import the library in (basically) the same way you are doing now. As I see it, we need to:

  1. Submit a PR to https://github.com/mattgwwalker/msg-extractor to include a setup.py and ask him to put his package on pypi. This will make it possible to install his package in the system python distribution. More on writing setup.py files here, but you can always use textract's setup.py as a template.
  2. Add msg-extractor to textract's python requirements file and rework the import statements now that msg-extractor is installed on the system. If its on pypi, this is easy. If not, you can always just add msg-extractor as an editable dependency, even from your fork of msg-extractor.
  3. Voilá!

Again, I'd be happy to work with you on this if you'd like or to just provide helpful pointers along the way if that is more your speed. Thanks again!

@anthonygarvan

This comment has been minimized.

Contributor

anthonygarvan commented Aug 6, 2014

@deanmalmgren - sure, makes sense. I'll let you know when I've pushed the changes to my msg-extractor fork, I'd appreciate it if you could check it out and make sure everything looks good. Thanks!

@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Aug 8, 2014

Sounds good, Anthony! Thanks for taking a stab at it!

dean malmgren | +1.312.884.9214 partner, data scientist | datascope

On Wed, Aug 6, 2014 at 6:05 PM, Anthony Garvan notifications@github.com
wrote:

@deanmalmgren https://github.com/deanmalmgren - sure, makes sense. I'll
let you know when I've push the changes to my msg-extractor fork, maybe you
can check it out and make sure everything looks good. Thanks!

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

@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Aug 22, 2014

Hey @anthonygarvan, sorry to do this to you mid-resolution, but I just thought I'd mention that I merged #39 which now switches to using a class-based set of parsers instead of the function based stuff that existed in v0.5.1. Have a look at the current parsers and let me know if you have any questions!

@anthonygarvan

This comment has been minimized.

Contributor

anthonygarvan commented Aug 22, 2014

Hey, no problem. I'm on two teams and they both have big releases in the
next month, so I won't have much bandwidth for a while. I hope to pick this
back up when the dust settles. If someone else wants to pick it up while
I'm busy, they should go for it.
On Aug 22, 2014 12:48 PM, "Dean Malmgren" notifications@github.com wrote:

Hey @anthonygarvan https://github.com/anthonygarvan, sorry to do this
to you mid-resolution, but I just thought I'd mention that I merged #39
#39 which now switches to
using a class-based set of parsers instead of the function based stuff that
existed in v0.5.1. Have a look at the current parsers and let me know if
you have any questions!


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

@deanmalmgren deanmalmgren force-pushed the deanmalmgren:master branch from df4664a to 515a12e Aug 25, 2014

@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Jun 15, 2015

@anthonygarvan I created a PR for msg-extractor (mattgwwalker/msg-extractor#4) and I'll hack together the bits of your solution to make sure everything works correctly. I'll include this in a separate pull request, but I wanted to thank you for your initial push to get this set up!

@deanmalmgren deanmalmgren referenced this pull request Jun 15, 2015

Merged

adding .msg support #87

@anthonygarvan

This comment has been minimized.

Contributor

anthonygarvan commented Jun 15, 2015

@deanmalmgren Thanks for taking the torch!

@deanmalmgren

This comment has been minimized.

Owner

deanmalmgren commented Jun 23, 2015

closed by #87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment