Move json_normalize to pd namespace by vishwakftw · Pull Request #27615 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation52 Commits24 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- Added a whatsnew entry
- Imported pandas.io.json.json_normalize in init.py
- closes Move json_normalize to pd namespace #27586
- whatsnew entry
cc: @WillAyd
- Added a whatsnew entry
- Imported pandas.io.json.json_normalize in init.py
Makes sense for the move. @TomAugspurger do you know how we've messaged things like this to users in the past to not import from pandas.io.json anymore?
I don't recall on messaging. Probably something "Accessing json_normalize from pandas.io has been deprecated and will be removed in a future version. Uses pandas.json_normalize instead."
But we'll need to make a pandas.io._json_normalize that has the implementation, and pandas.io.json_normalize will warn then call that. pandas/__init__.py will import io._json_normalize as json_normalize.
Thanks for review @jreback, please allow me to get back on this by the end of the weekend if that’s ok. I’ll also incorporate @TomAugspurger’s comment on deprecation as well.
Thanks! No rush, we’re still a ways out from 1.0.
On Jul 26, 2019, at 17:37, Vishwak Srinivasan ***@***.***> wrote: Thanks for review @jreback, please allow me to get back on this by the end of the weekend if that’s ok. I’ll also incorporate @TomAugspurger’s comment on deprecation as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
- json_normalize --> _json_normalize
- json_normalize inside io.json._normalize is a deprecated method
- update whatsnew docs
- update io docs
- fix broken test_api
Hello @vishwakftw! 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 2019-12-18 19:02:46 UTC
Can you update doc/source/reference/io.rst so that the reference docs for json_normalize are for the top-level?
It seems that build_table_scheme also is exposed. Would you be willing to do a similar change for it? Either here or as a followup.
Can you update doc/source/reference/io.rst so that the reference docs for json_normalize are for the top-level?
Yes, will do that.
It seems that build_table_scheme also is exposed. Would you be willing to do a similar change for it? Either here or as a followup.
Would you like me to expose build_table_schema in the pd namespace?
Possibly. I didn't closely follow the issue discussing json_nomalize, but we should be consistent. @jreback thoughts?
On Mon, Jul 29, 2019 at 9:54 AM Vishwak Srinivasan ***@***.***> wrote: Can you update doc/source/reference/io.rst so that the reference docs for json_normalize are for the top-level? Yes, will do that. It seems that build_table_scheme also is exposed. Would you be willing to do a similar change for it? Either here or as a followup. Would you like me to expose build_table_schema in the pd namespace? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27615?email_source=notifications&email_token=AAKAOIVMVRVD4WW6VZDGKPTQB4ADJA5CNFSM4IHFVMN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3A67OY#issuecomment-516026299>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIQ2DKANOQIP4S5GFA3QB4ADJANCNFSM4IHFVMNQ> .
- Update entry in reference/io.rst
I also just commented on the issue about build_table_schema (#27586), but I am not sure that that warrants a top-level function.
build_table_schema doesn't deserve a top level function as it's not generally useful for end users (it's one part of actually writing out in the table format)
It's unfortunate that json_normalize leaked publicly the way it is now but it certainly has usage and enhancement requests coming in for it, hence this PR
I wouldn't say "leaked publicly" though, right? That's the intentional, documented location for it.
On Wed, Jul 31, 2019 at 4:06 PM William Ayd ***@***.***> wrote: build_table_schema doesn't deserve a top level function as it's not generally useful for end users (it's one part of actually writing out in the table format) It's unfortunate that json_normalize leaked publicly the way it is now but it certainly has usage and enhancement requests coming in for it, hence this PR — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27615?email_source=notifications&email_token=AAKAOIUX3GSP7YXWHME5HYLQCH5DTA5CNFSM4IHFVMN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3IR2NQ#issuecomment-517020982>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOISWNHKHZYOLLMDUJ23QCH5DTANCNFSM4IHFVMNQ> .
Can’t speak to the entire history of intention but looks like was added alongside read_json in #10301 but never moved to the top level whenever read_json was In any case I don’t think whatever we do with this function needs to dictate what happens for build_table_schema
On Jul 31, 2019, at 2:09 PM, Tom Augspurger ***@***.***> wrote: I wouldn't say "leaked publicly" though, right? That's the intentional, documented location for it. On Wed, Jul 31, 2019 at 4:06 PM William Ayd ***@***.***> wrote: > build_table_schema doesn't deserve a top level function as it's not > generally useful for end users (it's one part of actually writing out in > the table format) > > It's unfortunate that json_normalize leaked publicly the way it is now > but it certainly has usage and enhancement requests coming in for it, hence > this PR > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#27615?email_source=notifications&email_token=AAKAOIUX3GSP7YXWHME5HYLQCH5DTA5CNFSM4IHFVMN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3IR2NQ#issuecomment-517020982>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAKAOISWNHKHZYOLLMDUJ23QCH5DTANCNFSM4IHFVMNQ> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27615?email_source=notifications&email_token=AAEU4UIDSHL3FAQU6J7LRO3QCH5RZA5CNFSM4IHFVMN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3ISDQI#issuecomment-517022145>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEU4UL5H3YJORQL5ITYKOTQCH5RZANCNFSM4IHFVMNQ>.
…nto json_normalize-to-pd
@vishwakftw can you check doc failure and merge master?
@WillAyd I can't seem to understand where the doc is failing. Could you help?
Hmm I guess the old build logs got cleared. Just restarted it so error should show again in say 20 minutes
@WillAyd I was unsure about how to deprecate on import, and was wondering if you had an idea (#27615).
not sure; this might be OK as is if you can get CI to pass will take another look
I'm not sure if the CI failures are relevant.
Looks like you need to run black on the changes to pass CI
I’ll take a look at the rest this weekend
…nto json_normalize-to-pd
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…nto json_normalize-to-pd
@WillAyd Sure, I've rebased and will ping you once green.
@WillAyd There is one failure, I'm not sure if it is related:
=================================== FAILURES ===================================
______________________________ test_scikit_learn _______________________________
[gw1] linux -- Python 3.6.9 /home/travis/miniconda3/envs/pandas-dev/bin/python
df = A
0 1
1 2
2 3
@pytest.mark.filterwarnings("ignore:can't:ImportWarning")
def test_scikit_learn(df):
> sklearn = import_module("sklearn") # noqa
pandas/tests/test_downstream.py:72:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas/tests/test_downstream.py:20: in import_module
return importlib.import_module(name)
../../../miniconda3/envs/pandas-dev/lib/python3.6/importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:994: in _gcd_import
???
<frozen importlib._bootstrap>:971: in _find_and_load
???
<frozen importlib._bootstrap>:955: in _find_and_load_unlocked
???
<frozen importlib._bootstrap>:665: in _load_unlocked
???
<frozen importlib._bootstrap_external>:678: in exec_module
???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
???
../../../miniconda3/envs/pandas-dev/lib/python3.6/site-packages/sklearn/__init__.py:75: in <module>
from .utils._show_versions import show_versions
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
"""
# License: BSD 3 clause
import platform
import sys
import importlib
> from ._openmp_helpers import _openmp_parallelism_enabled
E ImportError: dlopen: cannot load any more object with static TLS
../../../miniconda3/envs/pandas-dev/lib/python3.6/site-packages/sklearn/utils/_show_versions.py:12: ImportError
Should have been resolved with changes to CI yesterday. Looks like another merge conflict - if you clean up and repush should be green now I think
Thank you @WillAyd. Apologies for the delay in getting this merged.
proost pushed a commit to proost/pandas that referenced this pull request
proost pushed a commit to proost/pandas that referenced this pull request
AlexKirko pushed a commit to AlexKirko/pandas that referenced this pull request
