AlexMili / torch-dataframe

Utility class to manipulate dataset from CSV file
MIT License
67 stars 8 forks source link

Column order for fixin issue #5 #11

Closed gforge closed 8 years ago

gforge commented 8 years ago

Adds a numerically keyed table with the column names self.colum_order. Whenever outputting anything this table is traversed so that the order is always the same.

The fix also handles reading column order from a CSV and saving it in the correct order. The latter required basically that a large piece of csvigo had to be internalized.

This PR also adds a __pairs function (note that this should be __pairs__ for serialization issues but it isn't fixed until later branches) and changes the head/tail to use the sub for consistency.

As the current output is not very table-like I've added a __tostring method (should be __tostring__ and will be fixed in later branch) that outputs something that resembles a markdown table when working outside the iTorch environment. See tests/visual_output.lua for examples.

AlexMili commented 8 years ago

Great PR ! According to tests your custom function table.exact_length is the one who mess it up.

gforge commented 8 years ago

This error is rather strange as I can't replicate it on my local computer. In my more recent branches I've changed so that I skip the problematic __pairs() in favor of directly accessing the dataset

gforge commented 8 years ago

Added improved error messages - not sure what to make of them though - the errors make no sense as to why they would appear on Travis and not on my machine. Mind checking if you can replicate the errors on your local machine?

AlexMili commented 8 years ago

I tested on my machine the specific commits and nothing too. Your last commits didn't produce errors on Travis. Strange...

gforge commented 8 years ago

The problem seems to be the __pairs behaving in an unexpected way. Anyway, I've removed that so it shouldn't be a problem. I've also in the later PR's updated the Travis file to us th instead of lua, this could possibly be the issue.