Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: pd.show_versions: json.decoder.JSONDecodeError #39701

Closed
2 of 3 tasks
impredicative opened this issue Feb 9, 2021 · 8 comments · Fixed by #39766
Closed
2 of 3 tasks

BUG: pd.show_versions: json.decoder.JSONDecodeError #39701

impredicative opened this issue Feb 9, 2021 · 8 comments · Fixed by #39766
Labels
Bug Dependencies Required and optional dependencies good first issue
Milestone

Comments

@impredicative
Copy link

impredicative commented Feb 9, 2021

  • I have checked that this issue has not already been reported.

  • I have confirmed this bug exists on the latest version of pandas.

  • (optional) I have confirmed this bug exists on the master branch of pandas.


Note: Please read this guide detailing how to provide the necessary information for us to reproduce your bug.

Code Sample, a copy-pastable example

import contextlib, io, json
import pandas as pd

redirect = io.StringIO()
with contextlib.redirect_stdout(redirect):
    pd.show_versions(as_json=True)  # produces invalid json
pd_versions = redirect.getvalue()
pd_versions = json.loads(pd_versions)  # raises JSONDecodeError

Problem description

pd.show_versions(as_json=True) is supposed to print valid JSON, but it really doesn't. It is supposed to work as advertised but it doesn't. It prints invalid JSON that is not parsable. The following error is printed when attempting to parse it:

json.decoder.JSONDecodeError: Expecting property name enclosed in double quotes: line 1 column 2 (char 1)

This functionality is important because when running a job in the cloud, it is not feasible to print a multiline output. This is because every printed line has significant attached metadata. It is better to have valid JSON output instead in this case. The implementation of the show_versions function looks to be very hacky and poorly reviewed, given that it fails in this very basic way.

Expected Output

In the code example, json.loads(pd_versions) should have been able to parse the captured text.

Output of pd.show_versions()

INSTALLED VERSIONS

commit : 7d32926
python : 3.8.6.final.0
python-bits : 64
OS : Darwin
OS-release : 19.6.0
Version : Darwin Kernel Version 19.6.0: Tue Nov 10 00:10:30 PST 2020; root:xnu-6153.141.10~1/RELEASE_X86_64
machine : x86_64
processor : i386
byteorder : little
LC_ALL : None
LANG : None
LOCALE : en_US.UTF-8
pandas : 1.2.2
numpy : 1.19.4
pytz : 2020.4
dateutil : 2.8.1
pip : 21.0.1
setuptools : 50.3.2
Cython : None
pytest : 6.2.2
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : None
pandas_datareader: None
bs4 : None
bottleneck : None
fsspec : None
fastparquet : None
gcsfs : None
matplotlib : None
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : 3.0.0
pyxlsb : None
s3fs : None
scipy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
numba : None

@impredicative impredicative added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Feb 9, 2021
@simonjayhawkins
Copy link
Member

The implementation of the show_versions function looks to be very hacky and poorly reviewed, given that it fails in this very basic way.

PRs and contributions always welcome.

@simonjayhawkins simonjayhawkins added this to the Contributions Welcome milestone Feb 9, 2021
@lithomas1 lithomas1 removed the Needs Triage Issue that has not been reviewed by a pandas team member label Feb 10, 2021
@lithomas1
Copy link
Member

lithomas1 commented Feb 10, 2021

@impredicative I can reproduce this on master. Looks like

should be print(json.dumps(j)) instead.

@lithomas1 lithomas1 added the Dependencies Required and optional dependencies label Feb 10, 2021
@alexprincel
Copy link
Contributor

@impredicative I can reproduce this on master. Looks like


should be print(json.dumps(j)) instead.

I tested this locally and json.loads() now executes without error. I'm unsure about how to contribute but I'd be happy to try and submit a PR on this.

@alexprincel
Copy link
Contributor

It would really be better to return a JSON object. It makes no practical sense to print it. Printing the JSON didn't work to begin with, so no existing functionality will be broken by this change.

By JSON object do you mean a valid JSON dictionary as a Python string ? It would make this function either return a value or perform an action depending on wether as_json is True or False, which I do not think is desirable. Instead, it could be changed such that a value is printed and then returned. I do not think that this would break any existing code.

alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 11, 2021
@impredicative
Copy link
Author

I think it's fair to always print and also return the printed string. If as_json is False, print and return the printed string. If as_json is True, pretty print and return the valid json as a string. In this way the function's type signature would be consistent.

@lithomas1
Copy link
Member

@alexprincel Welcome to pandas 🚀 !
Contributing guidelines for PRs can be found here https://pandas.pydata.org/pandas-docs/stable/development/contributing.html if you need them. Before submitting a PR, can you also make sure to also add a test for this in pandas/tests/util/test_show_versions.py (something similar to what was in the OP)?
Thanks a lot.

@lithomas1
Copy link
Member

@impredicative
Can you open a separate issue for the proposed API changes to pandas.show_versions? I think it is unrelated to this bug.

@alexprincel
Copy link
Contributor

I submitted a PR for this. This is my first time doing a PR, ever. Please don't be shy on feedback if there are improvements to be made. Thanks!

alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 12, 2021
@lithomas1 lithomas1 modified the milestones: Contributions Welcome, 1.3 Feb 12, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 19, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 19, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 19, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 19, 2021
alexprincel added a commit to alexprincel/pandas that referenced this issue Feb 22, 2021
MarcoGorelli added a commit that referenced this issue Feb 22, 2021
* BUG: Fix pd.show_versions as_json invalid JSON (#39701)

* BUG: Separate multiple tests (39701)

* BUG: Various minor improvements on test_show_versions (#39701)

* BUG: Various test improvements to test_show_versions (#39701)

* BUG: Remove unused function (#39701)

* Update github issue number

Co-authored-by: Marco Gorelli <marcogorelli@protonmail.com>

Co-authored-by: Marco Gorelli <marcogorelli@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Dependencies Required and optional dependencies good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants