sensiblecodeio / databaker

Command line tool to convert spreadsheets to databases, made for the UK's Office for National Statistics.
Other
78 stars 13 forks source link

Move the bag overrides into fork of xypath #45

Open goatchurchprime opened 8 years ago

goatchurchprime commented 8 years ago

At line 182 of overrides.py # === Bag Overrides ======================================= is a set of functions that are actually member functions of xypath.Bag, where bag===self

Same for some of the xypath.Table functions, like xypath.Table.excel_ref

(But leave any of the functions that refer to dimension, as these don't belong here at all!)

Can they be moved to a fork of xypath? Moving stuff between independent repos is rather awkward, which is probably why it got implemented this way.

StevenMaude commented 8 years ago

I agree the code could benefit from being better organised.

In this case, the "bag overrides" actually just add methods to xypath.Bag rather than overriding existing ones. There are other changes in there, e.g. xypath.xypath.Table.from_messy_ which do monkey patch existing methods.

If the changes are only useful for Databaker, then Databaker feels like the best place to put them to me. The monkey patching probably comes under this. If some of the Bag additions are potentially generally useful, rather than just needed for Databaker, we could move those into xypath.

I also note some of the code in there requires Databaker, whereas xypath currently doesn't have that as a requirement.

Options:

Tidying it up within Databaker seems best to me, but I'm open to arguments to do something else.