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

Python3.9 compatible #85

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Python3.9 compatible #85

wants to merge 2 commits into from

Conversation

pancak3
Copy link
Contributor

@pancak3 pancak3 commented Feb 4, 2021

json.loads removed arg encoding since python 3.9, see 'https://docs.python.org/3.9/library/json.html#json.loads'

@Qumeric
Copy link

Qumeric commented Apr 13, 2021

I think

is_below_py39 = False
if sys.version_info.major < 3 and sys.version_info.minor < 9:
    is_below_py39 = True

Should be moved inside json_loads. Maybe import sys too.

Also, maybe encoding can be dropped even for Python 3.7 and 3.8?

@pancak3
Copy link
Contributor Author

pancak3 commented Apr 14, 2021

I think

is_below_py39 = False
if sys.version_info.major < 3 and sys.version_info.minor < 9:
    is_below_py39 = True

Should be moved inside json_loads. Maybe import sys too.

Also, maybe encoding can be dropped even for Python 3.7 and 3.8?

I think

is_below_py39 = False
if sys.version_info.major < 3 and sys.version_info.minor < 9:
    is_below_py39 = True

Should be moved inside json_loads. Maybe import sys too.

Also, maybe encoding can be dropped even for Python 3.7 and 3.8?

Excellent advice! I am going to change it. Thanks!

@kuznetsov-m
Copy link

I have the same problem. @pancak3, thanks for this fix! Сould you please merge this PR?

@pancak3
Copy link
Contributor Author

pancak3 commented May 31, 2022

I have the same problem. @pancak3, thanks for this fix! Сould you please merge this PR?

Haha this is really an old PR! I would like to help, but only authorized users are allowed to merge. Maybe you can try rebase in your local environment.

@kuznetsov-m
Copy link

I have the same problem. @pancak3, thanks for this fix! Сould you please merge this PR?

Haha this is really an old PR! I would like to help, but only authorized users are allowed to merge. Maybe you can try rebase in your local environment.

Yes, I merged the PR in my own repository. But I do not give up hope that an authorized user will merge this PR. I just didn't find a better user to mention in this PR 🙂

@pancak3
Copy link
Contributor Author

pancak3 commented May 31, 2022

They did not maintain this for a long time. This branch may be merged decades later :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants