Repository navigation
Made _guess avoid a couple cases in which Tables are instantiated and then ignored - #2892
Conversation
|
Haven't had a close look at this yet, but would it supersede #2880? |
|
Oh yeah, I guess it would. I suppose either approach works, although it's probably optimal to just avoid instantiation altogether. That's a separate issue from the |
|
This approach looks good to me. I'll look at the code more closely later, but on first inspection this approach is even better than the advertised "not instantiating a table". In fact it often avoids even reading the data for the table if the header doesn't parse into acceptable columns. Previously if you had a long CSV file then the basic reader would read the whole thing as a single column table with column name "a,b,c" and string data values like "1,2,3". From that perspective this is short-circuiting in a very good way (assuming my initial understanding of the code changes is correct). |
|
True, and that's definitely an advantage of this approach. Of course this still doesn't get around the fact that the data still has to be read and split into lines (although not split into fields) for each reader...now that I think about it, this can be easily avoided by reading the (potential) file in |
|
@AmrAS1 @taldcroft - what is the status of this PR? |
|
I don't have any more changes in mind, but maybe @taldcroft is interested in further review? |
|
@AmrAS1 - ok - btw, it looks like this needs rebasing |
20a7cf5 to
3b2fff0
Compare
|
I'm OK with this, but it looks to be failing a couple of |
3b2fff0 to
866a5c4
Compare
|
Oops, I made a mistake while rebasing. This should be fixed now--waiting on Travis. |
866a5c4 to
150eb6c
Compare
|
Ah, I see...there was an issue with the fast reader because |
150eb6c to
a9a70b1
Compare
a9a70b1 to
19b70fe
Compare
Made `_guess` avoid a couple cases in which Tables are instantiated and then ignored
It occurred to me that, in addition to adding a weakref to
Tableand thinking about @taldcroft's ideas in #2881, one immediate way to avoid overhead in io.ascii guessing is to avoid creating aTableif it will later be thrown out. For example,_apply_include_exclude_nameschecks to see if column names are valid after theTableis instantiated, so I moved the validation code to a methodcheck_column_nameswhich is called after header reading. Also, the issue in #2877 involved a check in_guessto see iflen(dat.colnames) <= 1, which can also be moved to the reading code before aTableis created. Not sure if adding an attributeguessingtoBaseReaderis the right way to go about it, but anyway I'm curious to see what others think.