ENH: Add file buffer validation to I/O ops #15894

gfyoung · 2017-04-04T21:33:40Z

Allows for more uniform handling of invalid file buffers to our read_* functions.
Adds a ton of new documentation to inference.py

Partially addresses #15895.

jreback · 2017-04-04T21:37:49Z

pandas/types/inference.py

+
+
+def is_file_like(obj):
+    file_attrs = ('read', 'write', 'seek', 'tell')


can you add a doc-string describing this. (and of course any others you want to document; prob should open an issue for that).

Isn't too hard to fix here in this PR. Will do.

also add a versionadded (for this one).

Got it. Will make sure.

jreback · 2017-04-04T21:38:50Z

pandas/types/inference.py

+        if not hasattr(obj, attr):
+            return False
+
+    if not is_iterator(obj):


so a file_like must be an iterator, but an iterator is not necessarily file_like right?

Correct. The reason is the following:

>>> from unittest import mock >>> m = mock.Mock() >>> hasattr(m, "read") True >>> hasattr(m, "__iter__") False

k, maybe add that statement (or describe). Remember these are actually user-level accessible :>

That is true. In the doc-string, I presume?

yep exactly. after this is merged I am going to create an issue to improve these doc-strings (novice level); the ones you don't do :>

~~That won't be necessary. A commit is being added here for that.~~

EDIT: Ah, I see you expanded to include more than just this file. Fair enough.

yeah should document them all ...

examples too!

jreback · 2017-04-04T21:39:43Z

lgtm.

jreback · 2017-04-04T22:29:37Z

pandas/io/excel.py

-        if _is_url(io):
+        url_passed = _is_url(io)
+
+        if url_passed:
            io = _urlopen(io)


might be simpler to have this in an if/elif

if _is_url(io): io = _urlopen(io) elif not isinstance(io, .....) ...

Good point. Fixed.

codecov · 2017-04-04T22:32:37Z

Codecov Report

Merging #15894 into master will decrease coverage by <.01%.
The diff coverage is 95.12%.

@@            Coverage Diff             @@
##           master   #15894      +/-   ##
==========================================
- Coverage   90.97%   90.97%   -0.01%     
==========================================
  Files         145      145              
  Lines       49496    49518      +22     
==========================================
+ Hits        45027    45047      +20     
- Misses       4469     4471       +2

Flag	Coverage Δ
#multiple	`88.73% <95.12%> (ø)`	⬆️
#single	`40.64% <75.6%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/types/api.py	`100% <ø> (ø)`	⬆️
pandas/io/excel.py	`79.73% <100%> (+0.03%)`	⬆️
pandas/io/common.py	`70.08% <90.9%> (+0.75%)`	⬆️
pandas/types/inference.py	`98.3% <96.42%> (-1.7%)`	⬇️
pandas/io/packers.py	`88.19% <0%> (-0.32%)`	⬇️
pandas/core/common.py	`91.03% <0%> (+0.34%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a37067...5a8f8da. Read the comment docs.

gfyoung · 2017-04-05T04:27:22Z

@jreback : Everything is green and ready to go.

jorisvandenbossche

Nice docstrings!

jorisvandenbossche · 2017-04-05T08:11:11Z

pandas/types/inference.py

-    # python 3 generators have __next__ instead of next
-    return hasattr(obj, 'next') or hasattr(obj, '__next__')
+    """
+    Check if the object is an iterable.


Maybe good to notice here that a string does not give True (although it is iterable, but not an iterator)

nor is a datetime, yes would be good to mention these counter-examples

jorisvandenbossche · 2017-04-05T08:17:01Z

pandas/types/inference.py

-            not isinstance(arg, string_and_binary_types))
+def is_list_like(obj):
+    """
+    Check if the object is list-like.


Maybe give an inexhaustive list of objects that are regarded as list-like? (lists, sets, arrays, Series, ...)

I would also add example with np.array(..)

mention that strings & datetimes are NOT is_list_like .

jreback · 2017-04-05T14:07:20Z

pandas/types/inference.py

+    3) seek
+    4) tell
+
+    In addition, the object must be iterable, as mock


this is assuming a little bit of knowledge here. I would just eliminate this paragraph.

Makes sense. Done.

jreback · 2017-04-05T14:09:48Z

pandas/types/inference.py

    except TypeError:
        return False
    else:
        return True


-def is_sequence(x):
+def is_sequence(obj):
+    """


I would note that this is less strict than is_list_like. Further impl wise we almost never use this one. I don't really recall what the actual diffferences are though. you can have a look thru the code base and see if you can come up with a good reason for this (IIRC its in the printing routines somewhere).

Actually, I think it's the other way around:

>>> i = iter([1, 2, 3]) >>> is_list_like(i) True >>> is_sequence(i) False

is_sequence checks for len and iter properties, whereas is_list_like just checks for the __iter__ property.

I think leaving both functions are fine for now, especially in the context of this PR.

yes, just ideally wanted to document this.

Closes pandas-devgh-15337.

Partially addresses pandas-devgh-15895.

gfyoung · 2017-04-05T17:12:57Z

@jreback , @jorisvandenbossche : Everything is green and ready to go.

jreback · 2017-04-05T19:18:46Z

thanks!

jreback reviewed Apr 4, 2017

View reviewed changes

jreback added Compat pandas objects compatability with Numpy or Python functions IO CSV read_csv, to_csv labels Apr 4, 2017

jreback added this to the 0.20.0 milestone Apr 4, 2017

jreback mentioned this pull request Apr 4, 2017

API: formalize the pandas IO API #15862

Closed

jreback reviewed Apr 4, 2017

View reviewed changes

gfyoung force-pushed the validate-file-like branch from cc3a9b6 to 3c477be Compare April 5, 2017 00:49

jorisvandenbossche reviewed Apr 5, 2017

View reviewed changes

gfyoung force-pushed the validate-file-like branch from 3c477be to ba08e6d Compare April 5, 2017 13:45

jreback reviewed Apr 5, 2017

View reviewed changes

gfyoung added 2 commits April 5, 2017 12:08

ENH: Add file buffer validation to I/O ops

81103f7

Closes pandas-devgh-15337.

DOC: Document all of inference.py

5a8f8da

Partially addresses pandas-devgh-15895.

gfyoung force-pushed the validate-file-like branch from ba08e6d to 5a8f8da Compare April 5, 2017 16:08

jreback closed this in e4e87ec Apr 5, 2017

gfyoung deleted the validate-file-like branch April 5, 2017 19:57

chris-b1 mentioned this pull request Apr 26, 2017

is_file_like requirements are too strict for boto3 S3 objects #16135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add file buffer validation to I/O ops #15894

ENH: Add file buffer validation to I/O ops #15894

gfyoung commented Apr 4, 2017 •

edited

Loading

jreback Apr 4, 2017

gfyoung Apr 4, 2017

jreback Apr 4, 2017

gfyoung Apr 4, 2017

jreback Apr 4, 2017

gfyoung Apr 4, 2017 •

edited

Loading

jreback Apr 4, 2017

gfyoung Apr 4, 2017

jreback Apr 4, 2017 •

edited

Loading

gfyoung Apr 4, 2017 •

edited

Loading

jreback Apr 4, 2017

jreback Apr 4, 2017

jreback Apr 4, 2017

jreback commented Apr 4, 2017

jreback Apr 4, 2017

gfyoung Apr 5, 2017

codecov bot commented Apr 4, 2017 •

edited

Loading

gfyoung commented Apr 5, 2017

jorisvandenbossche left a comment

jorisvandenbossche Apr 5, 2017

jreback Apr 5, 2017

gfyoung Apr 5, 2017

jorisvandenbossche Apr 5, 2017

jreback Apr 5, 2017

gfyoung Apr 5, 2017

jreback Apr 5, 2017

gfyoung Apr 5, 2017

jreback Apr 5, 2017

gfyoung Apr 5, 2017 •

edited

Loading

gfyoung Apr 5, 2017

jreback Apr 5, 2017

gfyoung commented Apr 5, 2017

jreback commented Apr 5, 2017



		def is_file_like(obj):
		file_attrs = ('read', 'write', 'seek', 'tell')

ENH: Add file buffer validation to I/O ops #15894

ENH: Add file buffer validation to I/O ops #15894

Conversation

gfyoung commented Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

gfyoung Apr 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Apr 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 4, 2017 • edited Loading

Codecov Report

gfyoung commented Apr 5, 2017

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Apr 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung commented Apr 5, 2017

jreback commented Apr 5, 2017

gfyoung commented Apr 4, 2017 •

edited

Loading

gfyoung Apr 4, 2017 •

edited

Loading

jreback Apr 4, 2017 •

edited

Loading

gfyoung Apr 4, 2017 •

edited

Loading

codecov bot commented Apr 4, 2017 •

edited

Loading

gfyoung Apr 5, 2017 •

edited

Loading