Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Feature/native source #57

Merged
merged 16 commits into from
Aug 8, 2016
Merged

Feature/native source #57

merged 16 commits into from
Aug 8, 2016

Conversation

roll
Copy link
Member

@roll roll commented Aug 7, 2016

It fixes #35

@coveralls
Copy link

coveralls commented Aug 7, 2016

Coverage Status

Coverage increased (+0.2%) to 88.956% when pulling 27c3057 on feature/native-source into 462d010 on master.

@roll
Copy link
Member Author

roll commented Aug 7, 2016

@zhenyab
please review

@@ -23,7 +20,9 @@ def detect_scheme(source):
For example `http` from `http://example.com/table.csv`

"""
if hasattr(source, 'read'):
if isinstance(source, list):
Copy link
Member Author

@roll roll Aug 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we need something smarter to work not only with lists but any iterables.
May be we should use NativeLoader for any non string sources (closing else).

Copy link
Member

@pwalsh pwalsh Aug 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import types

# stuff

if isinstance(source, (list, tuple, set, types.GeneratorType)):
   pass

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll test it tomorrow but it looks very good.

@coveralls
Copy link

coveralls commented Aug 7, 2016

Coverage Status

Coverage increased (+0.2%) to 88.956% when pulling 5dd5820 on feature/native-source into 462d010 on master.

@coveralls
Copy link

coveralls commented Aug 8, 2016

Coverage Status

Coverage increased (+0.2%) to 88.995% when pulling 5fed8c5 on feature/native-source into 462d010 on master.

@@ -53,7 +53,7 @@ def __emit_items(self):
prefix = '%s.item' % self.__path
items = ijson.items(self.__chars, prefix)
for item in items:
if isinstance(item, list):
if isinstance(item, (tuple, list)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roll why not set or generators?

Copy link
Member Author

@roll roll Aug 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pwalsh
Sorry I haven't answered it yet but I've checked types.GeneratorType it doesn't work for e.g. iter([1,2,3]). So I've decided to pass to NativeLoader/Parser all data types except file-like and strings - it's in detect_scheme/detect_format.

But check here is about rows:

[
    ('a','b','c'), # tuple
    (1,2,3), # tuple
    [1,2,3], # list
]

So almost there is no chance to have here generators/iterators and set is not ordered. It looks tuple-list could be enough. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right. good. thanks

@roll roll merged commit 65746c1 into master Aug 8, 2016
@roll roll removed the in progress label Aug 8, 2016
@roll roll deleted the feature/native-source branch August 9, 2016 13:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add native (list etc) sources support
4 participants