Skip to content

refactor: python js interface#381

Merged
hlky merged 6 commits into
Sygil-Dev:masterfrom
codedealer:refactor-frontend-1
Sep 1, 2022
Merged

refactor: python js interface#381
hlky merged 6 commits into
Sygil-Dev:masterfrom
codedealer:refactor-frontend-1

Conversation

@codedealer
Copy link
Copy Markdown
Collaborator

Some changes to the frontend folder following the feedback in #343.

css_and_js.py has been cleaned up. There is only one helper method call_SD that will call any method in js SD object you specify. Additionally it will pass any arguments into the method via object so the names of python arguments are translated to js directly.

  • Reliance on a shared property has been removed; every method now gets Gradio input argument as params.x.
  • Methods are allowed to return nullish values now (although those are converted to empty arrays internally for compatibility with Gradio frontend code).

@altryne
Copy link
Copy Markdown
Contributor

altryne commented Aug 31, 2022

I've opened a ticket to allow for global JS execution with Gradio folks, and would really love to wait and see what they come up with.
gradio-app/gradio#2141

Comment thread frontend/css_and_js.py Outdated
def call_SD(sd_method, **kwargs):
if "x" not in kwargs.keys():
kwargs["x"] = "x"
params = "{" + ",".join(f"{k}:{v}" for k, v in kwargs.items()) + "}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's some JSON formatter we could use here instead to get character escaping automatically? This is probably fine for now though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copying this from the other comment chain:

I would do recommend doing it like this:

import json

some_options = {
  "num": 123,
  "bool": True,
  "str": "this is a string"
}

json_str = json.dumps(some_options)

function_name = "some_function"

function_call_str = f"async (x) => {{ return await SD.{function_name}({{ ...{json_str}, x }}) ?? []; }}"

which results in:

async (x) => { return await SD.some_function({ ...{"num": 123, "bool": true, "str": "this is a string"}, x }) ?? [] }

which is essentially the same as this after the object spread is evaluated:

async (x) => { return await SD.some_function({ num: 123, bool: true, str: "this is a string", x }) ?? [] }

And the original types are retained without needing to manually add quotes around strings or convert True to true etc.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was resolved by 4da7cb6

@nderscore
Copy link
Copy Markdown
Contributor

Thanks for taking a look into this and addressing my feedback. This is a much cleaner abstraction 👏 🎉

I left a small comment, but no big concerns here 👍

@codedealer
Copy link
Copy Markdown
Collaborator Author

I've opened a ticket to allow for global JS execution with Gradio folks, and would really love to wait and see what they come up with. gradio-app/gradio#2141

@altryne my guy, I literally got this merged a few days back gradio-app/gradio#2108
As soon as 3.2 is released we shall have it.

@altryne
Copy link
Copy Markdown
Contributor

altryne commented Aug 31, 2022

I don't follow all their incoming issues haha so I didn't have any idea this is going to be possible, it's been a little hectic as you might imagine

I've opened a ticket to allow for global JS execution with Gradio folks, and would really love to wait and see what they come up with. gradio-app/gradio#2141

@altryne my guy, I literally got this merged a few days back gradio-app/gradio#2108 As soon as 3.2 is released we shall have it.

In that case, why merfe a refactor now when the above will get released we'll have a much cleaner interface?

@nderscore
Copy link
Copy Markdown
Contributor

In that case, why merfe a refactor now when the above will get released we'll have a much cleaner interface?

This change in gradio-app/gradio#2108 only impacts how this library code gets initialized - not how method calls work. The changes in this PR should still be relevant.

@codedealer
Copy link
Copy Markdown
Collaborator Author

Well the interface will stay mostly the same and this refactor was done by me mainly in preparation for the upcoming merge (I started it after my PR in gradio was accepted).

What it will allow us to do is to remove a hack used to instantiate SD global object cleanly and remove that travesty:

load_detector = gr.Number(value=0, label="Load Detector", visible=False)
load_detector.change(None, None, None, _js=js(opt))
demo.load(lambda x: 42, inputs=load_detector, outputs=load_detector)

I understand that it's been very chaotic recently and I don't know how closely you've been paying attention to the dev repo's codebase but we actually write js code in actual js files now. So it's not have been for nothing

@altryne
Copy link
Copy Markdown
Contributor

altryne commented Aug 31, 2022

write js code in actual js files now

That's an amazing upgrade on it's own, let's do it. the JS parts in python hurt my soul (and my vscode)

I understand that it's been very chaotic recently

Yeah, just a few short days, and tons of people asking for help or collaboration or access etc, good trouble.

@altryne
Copy link
Copy Markdown
Contributor

altryne commented Aug 31, 2022

@codedealer looks like this it?
#297
I should set up codeowners so I get tag automatically for everything /frontend related 🤔

@codedealer
Copy link
Copy Markdown
Collaborator Author

Yes, this PR builds on top of that one.

@altryne
Copy link
Copy Markdown
Contributor

altryne commented Aug 31, 2022

Catching up

Comment thread frontend/css_and_js.py Outdated
Comment thread frontend/frontend.py Outdated
@codedealer
Copy link
Copy Markdown
Collaborator Author

@altryne don't merge it yet :D

@altryne altryne self-requested a review August 31, 2022 19:40
@codedealer codedealer requested review from altryne and nderscore and removed request for altryne and nderscore August 31, 2022 20:12
@codedealer
Copy link
Copy Markdown
Collaborator Author

Clipboard api, gave me grief. Should be good now

@nderscore
Copy link
Copy Markdown
Contributor

updates in 4da7cb6 look great 👏

thanks again for updating 🙏

@codedealer codedealer deleted the refactor-frontend-1 branch September 7, 2022 10:22
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