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

JavaScript does not actually support named parameters #45

Closed
weaverba137 opened this issue Aug 20, 2020 · 6 comments
Closed

JavaScript does not actually support named parameters #45

weaverba137 opened this issue Aug 20, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@weaverba137
Copy link
Member

As far as I can tell, JavaScript does not support named parameters, which means that the functions defined in smooth_data.js are broken. Please correct me if I am wrong.

@armengau
Copy link
Collaborator

Indeed I realise that named parameters is not standard JS, at least !!

While implementing this heresy, I tried on https://www.w3schools.com/js/tryit.asp?filename=tryjs_function_return, and it seemed to work. What is even stranger is that the functions with named parameters in smooth_data.js worked ... with bokeh 1.3: to my understanding this issue appeared after switching to bokeh 2.1.

The correction you brought within the branch specutils apparently works fine.
In order not to depend on the branch merging, I implemented it word for word in my own branch scripts_n_doc, to be able to create html pages for target reinspection now.

So, for me this bug is fixed

@weaverba137
Copy link
Member Author

At this point it would be better to actually merge the PR, rather than risk creating new merge conflicts.

@armengau
Copy link
Collaborator

It's ok for me to merge now, I just thought you were waiting for Stephen's review.

Thanks for spotting that bug. Just yesterday evening I ran prospect ("old" version) with bokeh 2.1 for the first time, and indeed found it did not work (pbl with ivar_in in the console log...) I don't know if I would have easily found the bug by myself.

@weaverba137
Copy link
Member Author

I'm hoping Stephen can look at that today.

@sbailey
Copy link
Collaborator

sbailey commented Aug 24, 2020

Please proceed without me; both of your are much more up-to-speed on Prospect than me at this point and don't need to wait for my review/approvals. Please do update it as needed to work with bokeh 2.1. Thanks.

@weaverba137
Copy link
Member Author

Fixed in #43.

armengau added a commit that referenced this issue Aug 28, 2020
…umns, as widgetboxes become deprecated with bokeh 2.1. Small bug fix for VI_spectype default value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants