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

[Feature Request] API Calls like visualize_from_config can't process extensions. #84

Closed
vproHacks opened this issue Jun 13, 2024 · 7 comments · Fixed by #85
Closed

[Feature Request] API Calls like visualize_from_config can't process extensions. #84

vproHacks opened this issue Jun 13, 2024 · 7 comments · Fixed by #85
Assignees
Labels
back-end For issues where the root is mostly occurring on the back-end and not a specific adapter stat:awaiting model-navigator The issue is actively being worked on by our navigators type:feature Feature requests

Comments

@vproHacks
Copy link
Contributor

API Calls can be seen in src/server/package/src/model_explorer/apis.py, I'd assume a simple modification like adding the extensions keyword argument as seen in cmdline.py of the same parent directory would suffice.

@pkgoogle
Copy link
Contributor

Hi @vproHacks, we reviewed your PR and request -- There are probably additional changes needed, we would like to understand your use case better (why you want to make this change) in order for us to help guide you better.

@pkgoogle pkgoogle added stat:awaiting response back-end For issues where the root is mostly occurring on the back-end and not a specific adapter labels Jun 13, 2024
@vproHacks
Copy link
Contributor Author

Hey! I simply wanted to configure the model_explorer using the inbuilt model_explorer.config() tool. I couldn't find that there wasn't any way to run a python function to load up this configuration and add my own extension on top of it without having to initiate a subprocess call. I thought that this would be a clean addition on top of the visualize_from_config function to provide that functionality. So far this small change has worked with the following script I've set up:

model_explorer.visualize_from_config(
        config=model_explorer_config,
        no_open_in_browser=True,
        port=model_explorer_port,
        extensions=['my_adapter']
    )

@pkgoogle pkgoogle added type:feature Feature requests under discussion This issue is currently undergoing discussion to determine priority, complexity, and/or tractability and removed stat:awaiting response labels Jun 17, 2024
@pkgoogle
Copy link
Contributor

Hi @vproHacks, we are working on unit tests to be able to provide a harness to test out your changes. Thanks for your patience.

@pkgoogle pkgoogle added stat:awaiting model-navigator The issue is actively being worked on by our navigators and removed under discussion This issue is currently undergoing discussion to determine priority, complexity, and/or tractability labels Jun 21, 2024
@vproHacks
Copy link
Contributor Author

@yijie-yang @pkgoogle Any updates? It's been a while, just hoping that this doesn't get lost.

@yijie-yang
Copy link
Collaborator

Yes, thanks! We'll add more unit tests to the python package for the code health later. For this PR, it's simple and we can directly approve it.

@yijie-yang yijie-yang linked a pull request Jun 28, 2024 that will close this issue
yijie-yang pushed a commit that referenced this issue Jun 28, 2024
* added change to visualize_from_config

* Implemented for visualize API + Documentation.

* Final changes + Implementation for visualize_pytorch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end For issues where the root is mostly occurring on the back-end and not a specific adapter stat:awaiting model-navigator The issue is actively being worked on by our navigators type:feature Feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@yijie-yang @vproHacks @pkgoogle and others