-
Notifications
You must be signed in to change notification settings - Fork 3
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
Agave renderer #61
Agave renderer #61
Conversation
b21a91a
to
d335fd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty fancy! Should I make a non-abstract implimentation example or will you or we save for latter?
@@ -8,6 +8,7 @@ | |||
"dev": "pnpm -r --parallel run dev", | |||
"build": "pnpm -r build", | |||
"lint": "pnpm prettier --check . && pnpm eslint .", | |||
"lint:fix": "pnpm prettier --write . && pnpm eslint --fix .", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹
case UnknownEventAction.IGNORE: | ||
pass | ||
|
||
async def start_renderer(self, hypha_server_url, load_image_into_agave_fn, visibility="public"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this is moved in as a class function. Kind of liked it as a top level factory func, but no biggie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I can see it both ways, but since it references self
quite a bit, I made it a method.
I am renaming it to connect
, but suggestions are welcome on the naming.
|
||
await server.register_service({ | ||
"name": "Agave Renderer", | ||
"id": "agave-renderer", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumping the id
seemed to solve some of my connection problems, some of the time. Making this as configurable as hypha_server_url
might be convenent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 making a kwarg
|
||
"setup": self.setup, | ||
|
||
"loadImage": functools.partial(load_image_into_agave_fn, renderer), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderer
is coming from self.renderer
as a class function now I suppose.
Yes, great idea. Let's do it after we have another renderer implementation. Tracked: |
No description provided.