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

DEV: Added support for custom Notebook server base_url(s). Fixes #21. #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

snth
Copy link
Contributor

@snth snth commented Jun 23, 2015

Without this change show_grid() will not work when you have a custom prefix such as '/MYAPP' as part of your URL.

This change allows it to work on my installation which has a custom prefix. I hope to have preserved the functionality on installations where a custom prefix isn't present but I don't have an instance like that to test it myself.

This fixes issue #21.

@ssanderson
Copy link
Contributor

@snth thanks!. LGTM, assigning to @TimShawver for testing/merge.

@TimShawver
Copy link
Contributor

@snth thanks for this PR. I tried this locally and it doesn't work on IPython 3, because the NotebookApp configuration is separate from the kernel's configuration (so 'NotebookApp' in ipython.config always evaluates to false).

We think the right way to fix this is to add an additional configuration option to the show_grid function, similar to the existing remote_js=False option, that would allow the user to specify the base_url for the server.

Since base_url is a config option that you'd probably just want to set once rather than on every call to show_grid, you could do something like add a configure function that a user could call once with options like this: qgrid.configure(remote_js=False, base_url='my_base_url') The function could use a dictionary internally to keep track of the configured options, and use them as the default options for subsequent calls to show_grid.

Let me know what you think. I know automatically detecting the base_url would be nicer, but I had trouble figuring out how to do that on IPython 3. That being said I think doing this via configuration will be more maintainable in that we won't have to have different code for every version of IPython.

Thanks so much for your contributions.

@snth
Copy link
Contributor Author

snth commented Jun 30, 2015

@TimShawver Thanks for testing this on IPython 3. I decided to try this out myself on a Windows machine and also had problems. I eventually got something to work which works across IPython 2.x and 3.x but I believe the original behaviour may be a bug in the 3.x series.

I've posted it on StackOverflow here: http://stackoverflow.com/questions/31135008/how-does-one-access-the-configuration-from-inside-a-ipython-3-x-jupyter-notebo

@ssanderson
Copy link
Contributor

@snth the configuration of the notebook is intentionally not accessible from the IPython kernel (which is the process where the code in question is executing), because the kernels are meant to be compatible with multiple frontends, and the architecture of the kernel is such that it doesn't know whether it's connected to the notebook vs, e.g., the QTConsole or a terminal frontend.

I'm pretty sure there's no supported way of reliably auto-detecting the base URL, so it think the right thing to do is make it explicitly configurable by the user.

@snth
Copy link
Contributor Author

snth commented Jun 30, 2015

@ssanderson You're right, I hadn't thought about the kernel connecting to multiple frontends. The kernel is therefore not the right place to get this information. I think the Notebook JS API may know about this though and there might be a way to get it from there. I've updated my StackOverflow question to ask about this so let's see what comes back.

@dhirschfeld
Copy link

I was just bitten by this. Have just hard-coded the LOCAL_URL in my grid.py and it seems to work...

richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 26, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 27, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 31, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 31, 2021
richardlin047 added a commit to richardlin047/modin-spreadsheet that referenced this pull request Mar 31, 2021
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