Location component #1101

MarcSkovMadsen · 2020-02-16T07:26:12Z

I've created this initial contribution to start the discussion and work on #811.

I.e. i've started adding code to wrap the JS window.location api in order to easily let users bookmark and share links with search parameters corresponding to (parts of) the app state. I believe this is very power full. And this is something I wan't and need for my use cases.

Feel free to guide me (@philippjfr and others) on the architecture and requirements.

I've inserted some comments in the code where I would like some discussion.

…on-component

philippjfr · 2020-02-19T17:59:40Z

Very excited about this, sorry I haven't gotten around to reviewing this. Will swing back to it early next week once the Bokeh and HoloViews release sprinting are over.

…on-component

MarcSkovMadsen · 2020-02-23T14:55:12Z

A test of the Location pane fails. To me it seems like there is a bug in the panel code. I've tried to understand it without success. I would need some help from @philippjfr .

MASMA@PC70601 MINGW64 /c/repos/private/panel (location-component)
$ pytest 'panel\tests\widgets\test_location.py'
================================================== test session starts ==================================================
platform win32 -- Python 3.7.4, pytest-5.3.3, py-1.8.1, pluggy-0.13.1 -- c:\repos\private\panel\.venv\scripts\python.exe
cachedir: .pytest_cache
rootdir: C:\repos\private\panel, inifile: tox.ini
collected 1 item                                                                                                         

panel/tests/widgets/test_location.py::test_location FAILED                                                         [100%]

======================================================= FAILURES ========================================================
_____________________________________________________ test_location _____________________________________________________

document = <bokeh.document.document.Document object at 0x000000795D5FA508>
comm = Comm(id='cd51eec7f2944cfaac9a8d439129fab7', name='Comm00005')

    def test_location(document, comm):
        # given
    
        # When
        location = Location(href=HREF, name="Location")
    
>       widget = location.get_root(document, comm=comm)

panel\tests\widgets\test_location.py:19:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
panel\viewable.py:536: in get_root
    root = self._get_model(doc, comm=comm)
panel\widgets\base.py:92: in _get_model
    self._link_props(model, properties, doc, root, comm)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = Location(href='https://panel.holoviz.org...), model = Location(id='1002', ...)
properties = ['disabled', 'hash_', 'hostname', 'href', 'pathname', 'port', ...]
doc = <bokeh.document.document.Document object at 0x000000795D5FA508>, root = Location(id='1002', ...)
comm = Comm(id='cd51eec7f2944cfaac9a8d439129fab7', name='Comm00005')

    def _link_props(self, model, properties, doc, root, comm=None):
        ref = root.ref['id']
        if comm is None:
            for p in properties:
                if isinstance(p, tuple):
                    _, p = p
                model.on_change(p, partial(self._server_change, doc, ref))
        elif config.embed:
            pass
        else:
            on_msg = partial(self._comm_change, ref=ref)
            client_comm = state._comm_manager.get_client_comm(
                on_msg=on_msg, on_error=partial(self._on_error, ref),
>               on_stdout=partial(self._on_stdout, ref)
            )
E           TypeError: get_client_comm() got an unexpected keyword argument 'on_error'

panel\viewable.py:753: TypeError
=================================================== 1 failed in 0.39s ===================================================
(.venv)
MASMA@PC70601 MINGW64 /c/repos/private/panel (location-component)
$ pip show panel
Name: panel
Version: 0.7.1a2.post23+gf028675.dirty
Summary: A high level dashboarding library for python visualization libraries.
Home-page: http://pyviz.org
Author: PyViz developers
Author-email: developers@pyviz.org
License: BSD
Location: c:\repos\private\panel
Requires: bokeh, param, pyviz-comms, markdown, pyct
Required-by:

ceball · 2020-02-23T15:20:41Z

What version of pyviz_comms do you have, @MarcSkovMadsen? Just in case you don't have the latest release, note what was added to it (on my phone, and haven't been following this pr, so sorry if it's no related...)

…

On Sun, Feb 23, 2020 at 2:55 PM Marc Skov Madsen ***@***.***> wrote: A test of the Location pane fails. To me it seems like there is a bug in the panel code. I've tried to solve it without success. I would need some help from @philippjfr <https://github.com/philippjfr> . ***@***.*** MINGW64 /c/repos/private/panel (location-component) $ pytest 'panel\tests\widgets\test_location.py' ================================================== test session starts ================================================== platform win32 -- Python 3.7.4, pytest-5.3.3, py-1.8.1, pluggy-0.13.1 -- c:\repos\private\panel\.venv\scripts\python.exe cachedir: .pytest_cache rootdir: C:\repos\private\panel, inifile: tox.ini collected 1 item panel/tests/widgets/test_location.py::test_location FAILED [100%] ======================================================= FAILURES ======================================================== _____________________________________________________ test_location _____________________________________________________ document = <bokeh.document.document.Document object at 0x000000795D5FA508> comm = Comm(id='cd51eec7f2944cfaac9a8d439129fab7', name='Comm00005') def test_location(document, comm): # given # When location = Location(href=HREF, name="Location") > widget = location.get_root(document, comm=comm) panel\tests\widgets\test_location.py:19: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ panel\viewable.py:536: in get_root root = self._get_model(doc, comm=comm) panel\widgets\base.py:92: in _get_model self._link_props(model, properties, doc, root, comm) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = Location(href='https://panel.holoviz.org...), model = Location(id='1002', ...)properties = ['disabled', 'hash_', 'hostname', 'href', 'pathname', 'port', ...]doc = <bokeh.document.document.Document object at 0x000000795D5FA508>, root = Location(id='1002', ...)comm = Comm(id='cd51eec7f2944cfaac9a8d439129fab7', name='Comm00005') def _link_props(self, model, properties, doc, root, comm=None): ref = root.ref['id'] if comm is None: for p in properties: if isinstance(p, tuple): _, p = p model.on_change(p, partial(self._server_change, doc, ref)) elif config.embed: pass else: on_msg = partial(self._comm_change, ref=ref) client_comm = state._comm_manager.get_client_comm( on_msg=on_msg, on_error=partial(self._on_error, ref),> on_stdout=partial(self._on_stdout, ref) )E TypeError: get_client_comm() got an unexpected keyword argument 'on_error'panel\viewable.py:753: TypeError=================================================== 1 failed in 0.39s ***@***.*** MINGW64 /c/repos/private/panel (location-component)$ pip show panelName: panelVersion: 0.7.1a2.post23+gf028675.dirtySummary: A high level dashboarding library for python visualization libraries.Home-page: http://pyviz.orgAuthor: PyViz developersAuthor-email: ***@***.***: BSDLocation: c:\repos\private\panelRequires: bokeh, param, pyviz-comms, markdown, pyctRequired-by: — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1101?email_source=notifications&email_token=AAAAPCIG3RMP2TUQ47ODRL3REKE5DA5CNFSM4KWAPRWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMV5W4A#issuecomment-590076784>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAPCJF2ZNN4EYA2Z3EA3DREKE5DANCNFSM4KWAPRWA> .

MarcSkovMadsen · 2020-02-23T18:37:46Z

$ pip show pyviz_comms
Name: pyviz-comms
Version: 0.7.2
Summary: Bidirectional communication for the PyViz ecosystem.
Home-page: http://pyviz.org
Author: PyViz developers
Author-email:
License: BSD
Location: c:\repos\private\panel\.venv\lib\site-packages
Requires: param
Required-by:
(.venv)
```bash

MarcSkovMadsen · 2020-02-23T18:38:49Z

I've upgraded to pyviz_comms 0.7.3 and the test passes. Thanks @ceball

philippjfr · 2020-02-23T18:39:20Z

Right the latest version is 0.7.3 and is listed as a requirement.

MarcSkovMadsen · 2020-02-23T18:42:21Z

Thanks @philippjfr

What I did was git pull holoviz master to get the latest version of Panel Master into my feature branch. And I'm thinking whats the command I should use to automatically upgrade to the requirements that this version has? Or maybe just check if I have the required versions?

philippjfr · 2020-02-23T18:43:08Z

Probably best to rerun:

pip install -e .

after every pull, that should also rebuild changed models.

philippjfr · 2020-02-23T19:08:06Z

One consideration I was thinking about here is that the pn.state.window needs to change according to which user session is currently being processed. I think we will needs to create a pn.state._windows indexed by the bokeh Document and then the pn.state.window would be a property that uses pn.state.curdoc to look up the appropriate window.

The second consideration is how this model gets sent to the frontend in the first place. I think it'll be modeled on the State model which gets added as an additional Document root. Since we always want the Window model to be available we'll have to think about the correct place to do this.

MarcSkovMadsen · 2020-02-23T19:18:47Z

The alternative to adding window to state is that its something that the user actively should instantiate and add to his/ her Panel app. For Dash the Location component works like that.

I guess the window.location will be used be a minor number of users/ in a minor number of use cases.

MarcSkovMadsen · 2020-02-23T19:27:50Z

I can now start making the Bokeh Model .py+.ts and Panel widget .py file work.

The first issue is that the Bokeh location.ts model is instantiated with the correct values but the Panel widget location.py uses the python default values and not the location.ts default values.

How do I synchronize values from location.ts to location.py on instantation?

Bokeh Model location.ts constructed correctly

$ python -m panel serve 'panel\tests\models\test_location.py' --dev --show

Panel Model location.py not instantiated correctly

$ python -m panel serve 'panel\tests\widgets\test_location.py' --dev --show

I would like the parameter values to be set as defined in the url. They are just "".

MarcSkovMadsen · 2020-02-23T19:40:47Z

One more question is the api? What is the use case and requirements?

Your input to the below would be highly appreciated

I would say its to be able

get the full href and the individual components like hostname, pathname, protocol, port, search and hash.
set the pathname, search and hash only
specify whether the window should be refreshed (i.e. do a get request in an old style multipage app) or update the url only visually (single page app)

I don't think the location widget is a way to move away from the Panel app, i.e. to set the hostname, protocol or port. I can also see that Chrome does not allow this in some cases as it is considered insecure.

Then there is the question if the search and hash parameters on the Python side should have the same value as the window.location.search and window.location.hash, i.e. have a leading ? or # (as in js) or not. I think they should as this is easier to understand if you know the window.location.api.

I also don't think the user should be able to set the href value in the python widget. She should only be able to get it. This is to ease the implementation and testing.

$ python -m panel serve 'panel\tests\widgets\test_location.py' --dev --show

MarcSkovMadsen · 2020-02-25T04:18:11Z

Some of the parameters on the Location widget like href, port, protocol and hostname are readonly because they should not be changed by the user. They are there to be read only.

But being readonly raises an error when I set them on the Bokeh .ts model because they cannot be synced to the Panel .py widget.

One way to solve that would be to have private _href, _port, _protocol and _hostname parameters on the widget that is synced from .ts to .py and then I can use python to update the href, port, protocol and hostname parameters.

I would like some suggestions on the right way to solve this

$ python -m panel serve 'panel\tests\widgets\test_location.py' --dev --show
2020-02-25 04:58:39,571 Starting Bokeh server version 1.4.0 (running on Tornado 6.0.3)
2020-02-25 04:58:39,589 User authentication hooks NOT provided (default user enabled)
2020-02-25 04:58:39,594 Bokeh app running at: http://localhost:5006/test_location
2020-02-25 04:58:39,594 Starting Bokeh server with process id: 7216
2020-02-25 04:58:47,267 404 GET /favicon.ico (127.0.0.1) 0.00ms
2020-02-25 04:58:47,298 WebSocket connection opened
2020-02-25 04:58:47,300 ServerConnection created
2020-02-25 04:58:47,439 Exception in callback functools.partial(<bound method IOLoop._discard_future_result of <tornado.platform.asyncio.AsyncIOMainLoop object at 0x00000075FAA8B408>>, <Future finished exception=TypeError("Read-only parameter 'href' cannot be modified")>)
Traceback (most recent call last):
  File "C:\repos\private\panel\.venv\lib\site-packages\tornado\ioloop.py", line 743, in _run_callback
    ret = callback()
  File "C:\repos\private\panel\.venv\lib\site-packages\tornado\ioloop.py", line 767, in _discard_future_result
    future.result()
  File "C:\repos\private\panel\.venv\lib\site-packages\tornado\gen.py", line 748, in run
    yielded = self.gen.send(value)
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\server\session.py", line 70, in _needs_document_lock_wrapper
    result = yield yield_for_all_futures(func(self, *args, **kwargs))
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\server\session.py", line 191, in with_document_locked
    return func(*args, **kwargs)
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\document\document.py", line 1127, in wrapper
    return doc._with_self_as_curdoc(invoke)
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\document\document.py", line 1113, in _with_self_as_curdoc
    return f()
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\document\document.py", line 1126, in invoke
    return f(*args, **kwargs)
  File "C:\repos\private\panel\.venv\lib\site-packages\bokeh\document\document.py", line 916, in remove_then_invoke
    return callback(*args, **kwargs)
  File "C:\repos\private\panel\panel\viewable.py", line 794, in _change_event
    self._process_events(events)
  File "C:\repos\private\panel\panel\viewable.py", line 784, in _process_events
    self.param.set_param(**self._process_property_change(events))
  File "C:\repos\private\panel\.venv\lib\site-packages\param\parameterized.py", line 1417, in set_param
    setattr(self_or_cls, k, v)
  File "C:\repos\private\panel\.venv\lib\site-packages\param\parameterized.py", line 299, in _f
    instance_param.__set__(obj, val)
  File "C:\repos\private\panel\.venv\lib\site-packages\param\parameterized.py", line 301, in _f
    return f(self, obj, val)
  File "C:\repos\private\panel\.venv\lib\site-packages\param\parameterized.py", line 877, in __set__
    raise TypeError("Read-only parameter '%s' cannot be modified" % self.name)
TypeError: Read-only parameter 'href' cannot be modified

MarcSkovMadsen · 2020-02-25T04:37:36Z

I can now start making the Bokeh Model .py+.ts and Panel widget .py file work.

The first issue is that the Bokeh location.ts model is instantiated with the correct values but the Panel widget location.py uses the python default values and not the location.ts default values.

How do I synchronize values from location.ts to location.py on instantation?

Bokeh Model location.ts constructed correctly
$ python -m panel serve 'panel\tests\models\test_location.py' --dev --show
Panel Model location.py not instantiated correctly

$ python -m panel serve 'panel\tests\widgets\test_location.py' --dev --show

I would like the parameter values to be set as defined in the url. They are just "".

Solved - If I set the model parameters in the .ts initialize() function then they get transfered to the .py model.

MarcSkovMadsen · 2020-02-25T06:13:38Z

I've solved the readonly issue described above.

I used the workaround described above. It was a little cumbersome setting all the connections of for _href, _port, _protocol and _name. Fortunately i could keep the bokeh model as originally developedv via _rename attribute on the Panel model. I had to create an edit_readonly function inspired by the param.edit_constant function. It works. Would still like to know if there was an easier way to do this?

The widget now works (I believe) as I want it to.

Next steps

Clean up. Will remove from state file and do some linting clean up.
Add notebook documentation and example.
Fix things identified in first two steps.

MarcSkovMadsen · 2020-02-25T19:18:57Z

philippjfr · 2020-03-11T17:23:50Z

Was hoping to get this into 0.9 but punting for now.

philippjfr · 2020-03-13T11:48:13Z

@MarcSkovMadsen I'm going to close this in favor of #1150, which takes a different approach to this. I don't have permission to work on your branch so to make progress I made a separate branch.

A few comments on the style/formatting/encoding of the PR rather than the contents.

All the files you push are in DOS format, there is probably some setting in your editor to change that.
The Typescript files you develop use four space indentation, the standard we use is two spaces.
Until we decide on a formatting for Python files it would be good not to run your auto-formatter on these files as it introduces a lot of diff noise in the PR.

Anyway those are just some nitpicky comments. The actual contents of this PR were a great jumping off point.

Marc Skov Madsen added 3 commits February 16, 2020 07:07

added .vscode folder

6fa3b96

Merge branch 'master' of https://github.com/holoviz/panel into locati…

58ea93a

…on-component

boiler plate code to make things concrete

8a9b2a3

MarcSkovMadsen added status: WIP type: feature A major new feature labels Feb 16, 2020

MarcSkovMadsen added this to the v0.9.0 milestone Feb 16, 2020

MarcSkovMadsen requested a review from philippjfr February 16, 2020 07:26

MarcSkovMadsen self-assigned this Feb 16, 2020

MarcSkovMadsen mentioned this pull request Feb 16, 2020

Add functionality to update browser url with parameters in order for users bookmark their views and settings #811

Closed

Marc Skov Madsen added 3 commits February 22, 2020 06:17

The bokeh model can now instantiate with the correct values

0470231

connected signals and improved tests

c9130e6

Merge branch 'master' of https://github.com/holoviz/panel into locati…

03b9725

…on-component

got test_model.py for widget working by upgrading pyviz_comms to 0.7.3

2db06dc

Marc Skov Madsen added 4 commits February 24, 2020 05:53

added many tests. added readonly and regex to Location class

686afa3

improved regex and associated testing

bc1bcd5

added regex and tests

837c994

more work

8370fd2

fixed regex error on pathname

c353112

fixed init and readonly problems

d6a8a91

Marc Skov Madsen added 2 commits February 25, 2020 20:13

renamed refresh to reload

d720c98

added first iteration of workbook - not fully functional yet

e1a4ccf

philippjfr modified the milestones: v0.9.0, v1.0.0 Mar 11, 2020

philippjfr mentioned this pull request Mar 13, 2020

Location component #1150

Merged

philippjfr closed this Mar 13, 2020

philippjfr removed this from the v1.0.0 milestone Apr 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Location component #1101

Location component #1101

MarcSkovMadsen commented Feb 16, 2020

philippjfr commented Feb 19, 2020

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

ceball commented Feb 23, 2020 via email

MarcSkovMadsen commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

philippjfr commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020

philippjfr commented Feb 23, 2020 •

edited

Loading

philippjfr commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 25, 2020

MarcSkovMadsen commented Feb 25, 2020

Bokeh Model location.ts constructed correctly

Panel Model location.py not instantiated correctly

MarcSkovMadsen commented Feb 25, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 25, 2020

philippjfr commented Mar 11, 2020

philippjfr commented Mar 13, 2020

Location component #1101

Location component #1101

Conversation

MarcSkovMadsen commented Feb 16, 2020

philippjfr commented Feb 19, 2020

MarcSkovMadsen commented Feb 23, 2020 • edited Loading

ceball commented Feb 23, 2020 via email

MarcSkovMadsen commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020 • edited Loading

philippjfr commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020

philippjfr commented Feb 23, 2020 • edited Loading

philippjfr commented Feb 23, 2020

MarcSkovMadsen commented Feb 23, 2020 • edited Loading

MarcSkovMadsen commented Feb 23, 2020 • edited Loading

Bokeh Model location.ts constructed correctly

Panel Model location.py not instantiated correctly

MarcSkovMadsen commented Feb 23, 2020 • edited Loading

MarcSkovMadsen commented Feb 25, 2020

MarcSkovMadsen commented Feb 25, 2020

Bokeh Model location.ts constructed correctly

Panel Model location.py not instantiated correctly

MarcSkovMadsen commented Feb 25, 2020 • edited Loading

MarcSkovMadsen commented Feb 25, 2020

philippjfr commented Mar 11, 2020

philippjfr commented Mar 13, 2020

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

philippjfr commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 23, 2020 •

edited

Loading

MarcSkovMadsen commented Feb 25, 2020 •

edited

Loading