search

okfn / messytables

Tools for parsing messy tabular data. This is now superseded by https://github.com/frictionlessdata/tabulator-py
http://messytables.readthedocs.io/
387 stars 110 forks source link

Remove openpyxl, use XLSTableSet for XLSX files #84

Closed fawkesley closed 11 years ago

fawkesley commented 11 years ago

Phase 1 of 2 for completely removing openpyxl and using XLSTableSet instead. (Phase 2 will actually remove the dependency and excelx.py, then you won't be able to reference XLSXTableSet)

If you always use any_tableset it'll just work correctly - you'll now get back an XLSTableSet instead of an XLSXTableSet.

I've left the latter in with a DeprecationWarning (and test) in order to remain compatible with code written with explicity XLSXTableSet.

I'm feeling like we should encourage people towards only using any_tableset (perhaps with an argument to override force the type detection). It's quite awkward that currently our users are needlessly coupling to our class naming convention. Unless I've missed a use-case - any compelling reasons to allow that?

Not ready to merge yet I suspect. Closes #83

fawkesley commented 11 years ago

Something tells me I might benefit from installing Python 2.6 locally ;)

domoritz commented 11 years ago

@paulfurley I marked this wip.

fawkesley commented 11 years ago

@domoritz Agreed about helper function. Nicer to use assert_is_instance when available as its failure output is better than assert_true(isinstance(..))

domoritz commented 11 years ago

About the deprecation. Can we just use the XLSTableSet instead of XLSXTableSet? So, something like XLSXTableSet = XLSTableSet. This way we can remove the code but still be compatible.

Also, would you like to make the deprecation warnings consistent across messytables? We have another deprecated warning here: https://github.com/okfn/messytables/blob/master/messytables/core.py#L159 but it should also use warning, I guess.

fawkesley commented 11 years ago

OK let's be brutal and link XLSXTableSet to XLSTableSet and forget the DeprecationWarning.

I think separately we should try and wean people off constructing TableSet directly - I'll make an issue.

frabcus commented 11 years ago

Yeah - I personally think we should encourage the use of "any". Kind of the point of the library is to not care about the format.

I'd have a default constructor something like messytables.TableSet() which used "any".

On Wed, Aug 07, 2013 at 06:00:21AM -0700, paulfurley wrote:

OK let's be brutal and link XLSXTableSet to XLSTableSet and forget the DeprecationWarning.

I think separately we should try and wean people off constructing TableSet directly - I'll make an issue.

Reply to this email directly or view it on GitHub: https://github.com/okfn/messytables/pull/84#issuecomment-22249036

domoritz commented 11 years ago

Let me know when this is ready for final review.

fawkesley commented 11 years ago

@domoritz Ready.