Fix issue #36271 to disambiguate json string #36273

tbachlechner · 2020-09-10T18:19:56Z

pd.read_json() fails currently for strings that look like fsspec_url and contain "://". adding another condition to fix this at least in most cases

closes BUG: since pandas==1.1.0 pd.read_json() fails for strings that look similar to fsspec_url #36271
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pd.read_json() fails currently for strings that look like fsspec_url and contain "://". adding another condition to fix this at least in most cases

pep8speaks · 2020-09-10T18:20:00Z

Hello @tbachlechner! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-11 15:02:53 UTC

WillAyd · 2020-09-10T18:37:54Z

Does this close an issue? Can you add a test?

…andas-dev#36271

WillAyd · 2020-09-10T21:11:47Z

pandas/io/common.py

@@ -161,6 +161,7 @@ def is_fsspec_url(url: FilePathOrBuffer) -> bool:
    return (
        isinstance(url, str)
        and "://" in url
+        and not " " in url


@martindurant

A space is allowed in URLs for at least some storage systems

Thanks, any other ideas how to fix #36271 ? All my json strings with URLs in them receive errors and I downgraded to pandas==1.0.5

So, a space isn't necessary for valid JSON either. Perhaps the only way to tell is to try to parse the string?

I updated the PR to reflect what you say, I'm now trying to parse the string as json and if it succeeds, it's not a URL. How about that?

Sounds reasonable to me - I don't expect any URL is also valid JSON. You could conceivable make a regex to check for JSON-ness ("^\s*[\[{]" maybe?), but probably calling JSON decode is faster and more correct.

(I simply didn't know that you can pass a string directly to read_json, as opposed to a StringIO file-like)

martindurant · 2020-09-11T12:58:01Z

pandas/io/common.py

@@ -161,6 +176,7 @@ def is_fsspec_url(url: FilePathOrBuffer) -> bool:
    return (
        isinstance(url, str)
        and "://" in url
+        and not is_json(url)


For pandas maintainers: should this check only be done within read_json (which will presumably call json.loads anyway, repeating the work) ? Are there any other similar cases that the string passed might be handled as a path, or decoded directly, that I might have missed?

read_json doesn't call json.loads

@tbachlechner if you really think t his out to be caught then i suppose a 'better' check instead of and "://" in url could be done (e.g. a regex)

jreback · 2020-09-13T23:18:12Z

pandas/io/common.py

@@ -161,6 +176,7 @@ def is_fsspec_url(url: FilePathOrBuffer) -> bool:
    return (
        isinstance(url, str)
        and "://" in url
+        and not is_json(url)


read_json doesn't call json.loads

jreback · 2020-09-13T23:19:56Z

pandas/io/common.py

+def is_json(url: FilePathOrBuffer) -> bool:
+    """
+    Returns true if the given string looks like
+    something json.loads can handle


we cannot do this generally as its completely non-performant

jreback · 2020-09-13T23:20:55Z

pandas/io/common.py

@@ -161,6 +176,7 @@ def is_fsspec_url(url: FilePathOrBuffer) -> bool:
    return (
        isinstance(url, str)
        and "://" in url
+        and not is_json(url)


@tbachlechner if you really think t his out to be caught then i suppose a 'better' check instead of and "://" in url could be done (e.g. a regex)

tbachlechner · 2020-09-17T17:58:32Z

@jreback yes, this ought to be caught, pd.read_json() is claimed to accept a json string, but it fails for any json string containing ://- which is a lot of json strings. For now we pinned the version pandas==1.0.5. I'm busy shipping products, but I'll see if I get to a fix at some point. The underlying issue though is that is_fsspec_url() should not be returning True when it is not a URL, removing json is just a hack that probably does not fix other problems that likely exist.

martindurant · 2020-09-17T18:11:36Z

I agree that is_fsspec_url() should be better, and clearly is failing for this JSON case.
Does the regex I suggested earlier ( "^\s*[\[{]" ) not suffice?

jbrockmendel · 2020-09-23T20:40:35Z

mypy complaint

github-actions · 2020-10-24T00:16:44Z

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

twoertwein · 2020-11-06T01:05:00Z

are there opinions about moving read_json(json_string) into a function like parse_json(json_string) for an API-breaking 1.2+ release (are there more read_ functions that would need read_ vs parse_)? Something like:

def parse_json(json:str, **kwargs) -> DataFrame:
    buffer = StringIO(json)
    data = read_json(buffer, **kwargs)
    buffer.close()
    return data

and a deprecation warning for 1.2 when passing a json string to read_json.

edit:
read_excel has the same 'issue', it accepts bytes.

jbrockmendel · 2020-11-06T02:53:23Z

are there opinions about moving read_json(json_string) into a function like parse_json(json_string)

IIRC there was a discussion at some point of making a read_jsons mirroring the stdlib's json.load vs json.loads (or maybe the discussion was about pickle?)

jreback · 2020-11-06T04:07:47Z

it wouldn't be unreasonable to deprecate passing a str (that is not a path) to read_josh and instead only accept a StringIO

i don't think this is much of any issue for pickle as it needs bytes and so str always means a file

excel i think is the same

only read_josh accepts str as an actual transmission format (read_csv as well of course is similar but i think we are restrictive there already)

twoertwein · 2020-11-07T21:13:31Z

pandas/io/common.py

+    if json_pattern.match(url):
+        return True
+    else:
+        return False


can be simplified return json_pattern.match(url) is not None

thanks, I committed your suggestion

twoertwein · 2020-11-13T22:55:39Z

pandas/io/common.py

+    Returns true if the given string looks like
+    json
+    """
+    json_pattern = re.compile(r"^\s*[\[{]")


probably need something like the following for typing

if not isinstance(url, str): return False

The pre-commit is complaining about too many new lines.

jreback · 2020-12-29T20:44:16Z

closing as stale. if you would like to continue, pls ping.

Fix issue pandas-dev#36271 to disambiguate json string

8c40f23

pd.read_json() fails currently for strings that look like fsspec_url and contain "://". adding another condition to fix this at least in most cases

WillAyd added the IO JSON read_json, to_json, json_normalize label Sep 10, 2020

BUG: pd.read_json() fails for strings that look similar to fsspec_url p…

e5777f4

…andas-dev#36271

WillAyd reviewed Sep 10, 2020

View reviewed changes

tbachlechner added 6 commits September 10, 2020 14:22

fixed duplicate test_is_fsspec_url()

ed49bfe

Update common.py

0d7e063

check if url is a json string

3706d18

Test test_is_fsspec_url() interprets json as URL

210d659

Update test_common.py

6b2c98c

Update common.py

6019978

martindurant reviewed Sep 11, 2020

View reviewed changes

jreback requested changes Sep 13, 2020

View reviewed changes

use regex to check for json in url

def1881

github-actions bot added the Stale label Oct 24, 2020

twoertwein reviewed Nov 7, 2020

View reviewed changes

simplify is_json

943f644

twoertwein reviewed Nov 13, 2020

View reviewed changes

jreback closed this Dec 29, 2020

steve-mavens mentioned this pull request Sep 16, 2021

BUG: since pandas==1.1.0 pd.read_json() fails for strings that look similar to fsspec_url #36271

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue #36271 to disambiguate json string #36273

Fix issue #36271 to disambiguate json string #36273

tbachlechner commented Sep 10, 2020 •

edited

Loading

pep8speaks commented Sep 10, 2020 •

edited

Loading

WillAyd commented Sep 10, 2020

WillAyd Sep 10, 2020

martindurant Sep 10, 2020

tbachlechner Sep 10, 2020

martindurant Sep 10, 2020

tbachlechner Sep 10, 2020

martindurant Sep 11, 2020

martindurant Sep 11, 2020

jreback Sep 13, 2020

jreback Sep 13, 2020

jreback Sep 13, 2020

jreback Sep 13, 2020

jreback Sep 13, 2020

tbachlechner commented Sep 17, 2020

martindurant commented Sep 17, 2020

jbrockmendel commented Sep 23, 2020

github-actions bot commented Oct 24, 2020

twoertwein commented Nov 6, 2020 •

edited

Loading

jbrockmendel commented Nov 6, 2020

jreback commented Nov 6, 2020

twoertwein Nov 7, 2020

tbachlechner Nov 11, 2020

twoertwein Nov 13, 2020

jreback commented Dec 29, 2020

Fix issue #36271 to disambiguate json string #36273

Fix issue #36271 to disambiguate json string #36273

Conversation

tbachlechner commented Sep 10, 2020 • edited Loading

pep8speaks commented Sep 10, 2020 • edited Loading

Comment last updated at 2020-11-11 15:02:53 UTC

WillAyd commented Sep 10, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbachlechner commented Sep 17, 2020

martindurant commented Sep 17, 2020

jbrockmendel commented Sep 23, 2020

github-actions bot commented Oct 24, 2020

twoertwein commented Nov 6, 2020 • edited Loading

jbrockmendel commented Nov 6, 2020

jreback commented Nov 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 29, 2020

tbachlechner commented Sep 10, 2020 •

edited

Loading

pep8speaks commented Sep 10, 2020 •

edited

Loading

twoertwein commented Nov 6, 2020 •

edited

Loading