CLN: use f-string for JSON related files #30430
Conversation
pandas/io/json/_json.py
Outdated
| "JSON data had unexpected key(s): {bad_keys}".format( | ||
| bad_keys=pprint_thing(bad_keys) | ||
| ) | ||
| f"JSON data had unexpected key(s): {pprint_thing(bad_keys)}" |
There was a problem hiding this comment.
Not sure if pprint_thing is still needed - can you check?
There was a problem hiding this comment.
I'm unfamiliar with the specifics of pprint_thing and where and when it should be used. Could you let me know why you suspect it might no longer be needed, and I'll investigate.
There was a problem hiding this comment.
pprint_thing is partially left over from py2/py3 compat, but partially for pretty-pretting nested objects. In tthis case, bad_keys is defined a couple lines up and is a str, so pprint_thing is definitely not needed here.
There was a problem hiding this comment.
Thanks for the clarification. I removed both the import and the call to pprint_thing in that module
pandas/tests/io/json/test_pandas.py
Outdated
| f"{spaces}}},\n" | ||
| f'{spaces}"b":{{\n' | ||
| f'{spaces}{spaces}"0":"bar",\n' | ||
| f'{spaces}{spaces}"1":"qux"\n' |
There was a problem hiding this comment.
Can we just use double quotes opposed to mixing?
There was a problem hiding this comment.
the outer quotes all use " now
There was a problem hiding this comment.
Now I remember, the quotes actually became mixed as a result of running black pandas. I'd originally used ' for the outer quotes on each line. But black seems to prefer " as the outer when there are no quote literals inside the quote.
Changing all to outer quotes to " and using ' as the inner is even more problematic, as that causes the test to fail, thus I've changed it back to using ' as the outer quote for the time being.
Running black again as the CI is prompting me to do will once again result in mixed quotes. So please let me know what's the best practise here: consistent quoting or black-style?
There was a problem hiding this comment.
Just keep the original triple quoted string as is and prefix with an f
There was a problem hiding this comment.
Good point, I changed it back
pandas/io/json/_normalize.py
Outdated
| @@ -328,8 +328,7 @@ def _recursive_extract(data, path, seen_meta, level=0): | |||
|
|
|||
| if k in result: | |||
| raise ValueError( | |||
| "Conflicting metadata name {name}, " | |||
| "need distinguishing prefix ".format(name=k) | |||
| f"Conflicting metadata name {k}, " "need distinguishing prefix " | |||
There was a problem hiding this comment.
@jyscao an extra pair of parentheses got in here and needs to be removed. this sometimes happens when running black
There was a problem hiding this comment.
By parentheses, I'm guessing you mean extra pair of "? If that's the case, they have been removed.
| @@ -100,7 +100,7 @@ def assert_frame_equal(self, left, right, *args, **kwargs): | |||
| check_names=kwargs.get("check_names", True), | |||
| check_exact=kwargs.get("check_exact", False), | |||
| check_categorical=kwargs.get("check_categorical", True), | |||
| obj="{obj}.columns".format(obj=kwargs.get("obj", "DataFrame")), | |||
| obj=f"{kwargs.get('obj', 'DataFrame')}.columns", | |||
There was a problem hiding this comment.
can you take the kwargs.get out to a separate line
There was a problem hiding this comment.
Since keyword arguments are passed to tm.assert_index_equal, I didn't really know where best to put the new line for the kwargs.get call, so for the time being I put it before the call to tm.assert_index_equal. Also I'm not too sure if obj_type is a good binding name for this value. Please review this change.
pandas/tests/io/json/test_pandas.py
Outdated
| default_handler=str | ||
| ) == '{{"A":{{"0":"{hex}"}},"B":{{"0":1}}}}'.format(hex=hexed) | ||
| assert ( | ||
| df_nonprintable.to_json(default_handler=str) == f'{{"A":{{"0":"{hexed}"}}}}' |
There was a problem hiding this comment.
pls split into lines:
result = df_nonprintable...
expected = f'...'
assert result == expected
There was a problem hiding this comment.
Done. I also split the test below this one into 3 lines.
|
Thanks @jyscao |
* CLN: use f-string for JSON related files * Apply black style * Missed one... * Use double-quotes for expected in test_to_json_indent * Add the f in f-string * Use correct multiplier (100k not 10) for "bar" * Add back missing closing paren * Use single quote as the outer quotes * Remove pprint_thing usage and import * Keep use f""" for expected * Split two tests into 3 lines each * Remove paren around string * Remove extra pair "" in string * Move kwargs.get onto own line
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff