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

Reorganize code structure, fix client import bug #269

Merged
merged 5 commits into from
Nov 8, 2023

Conversation

mrwyattii
Copy link
Contributor

@mrwyattii mrwyattii commented Nov 8, 2023

This PR is a first step to better organize the MII code. Major changes include:

  • Creating an api.py file for user-facing functions
  • Moving client.py and server.py into backend/ (this also resolves a bug where we previously did from .client import client in our __init__.py file, which caused the mii.client submodule to be replaced with the mii.client function)
  • Moving models.py and tokenizers.py into modeling/
  • Moving task_methods.py under grpc_related/ (Because it is only used with the persistent GRPC deployment)

@jeffra

@jeffra jeffra requested a review from tohtana November 8, 2023 02:01
Copy link
Contributor

@jeffra jeffra left a comment

Choose a reason for hiding this comment

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

This is great, thank you @mrwyattii. @tohtana please take a look as well but otherwise looks good to me.

mii/api.py Outdated Show resolved Hide resolved
mii/api.py Show resolved Hide resolved
Copy link
Contributor

@tohtana tohtana left a comment

Choose a reason for hiding this comment

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

It looks good to me. I left small comments to share my thought, but I don't think we need to address them in this PR

@mrwyattii mrwyattii merged commit 83c4997 into main Nov 8, 2023
3 checks passed
@mrwyattii mrwyattii deleted the mrwyattii/fix-client-import-bug branch November 8, 2023 19:38
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.

3 participants