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

remove __version__ from cli.py #323

Merged
merged 8 commits into from
Feb 20, 2025

Conversation

igorsyl
Copy link
Contributor

@igorsyl igorsyl commented Feb 16, 2025

Fixes #241

@igorsyl igorsyl force-pushed the fix/igorsyl/version branch from f0e7a39 to 12cdebe Compare February 16, 2025 21:29
@igorsyl igorsyl force-pushed the fix/igorsyl/version branch from 12cdebe to 5325871 Compare February 16, 2025 21:29
@igorsyl igorsyl marked this pull request as ready for review February 18, 2025 14:37
@thewhaleking
Copy link
Contributor

We actually want to keep the __version__ in __init__.py and remove it from cli.py.
Additionally, let's move the e2e changes to a separate PR.

@igorsyl
Copy link
Contributor Author

igorsyl commented Feb 18, 2025

We actually want to keep the __version__ in __init__.py and remove it from cli.py. Additionally, let's move the e2e changes to a separate PR.

I removed the version string and asserts from cli.py and included them in __init__.py. I need to delay the import of __version__ in cli.py to avoid a circular import dependency. An alternative solution to avoid using delay imports is to use a new version.py file to keep the version string. cc @thewhaleking

@igorsyl igorsyl requested a review from thewhaleking February 19, 2025 14:25
@thewhaleking
Copy link
Contributor

We actually want to keep the __version__ in __init__.py and remove it from cli.py. Additionally, let's move the e2e changes to a separate PR.

I removed the version string and asserts from cli.py and included them in __init__.py. I need to delay the import of __version__ in cli.py to avoid a circular import dependency. An alternative solution to avoid using delay imports is to use a new version.py file to keep the version string. cc @thewhaleking

That's a good point. Let's move this to a new version.py file. Think that's gonna be a better solution than deferred imports.

@igorsyl igorsyl force-pushed the fix/igorsyl/version branch from 7f5bed0 to cea472e Compare February 19, 2025 20:41
@igorsyl
Copy link
Contributor Author

igorsyl commented Feb 19, 2025

Moved version string into new version.py file. cc @thewhaleking

@igorsyl
Copy link
Contributor Author

igorsyl commented Feb 20, 2025

@thewhaleking it appears the run-tests job is broken due to an unrelated reason to this PR. I can look into the issues in a separate PR.

@thewhaleking
Copy link
Contributor

thewhaleking commented Feb 20, 2025

@thewhaleking it appears the run-tests job is broken due to an unrelated reason to this PR. I can look into the issues in a separate PR.

I think this is actually due to a change on testnet (which is what we use to run the e2e tests). Checking into it more in-depth now, but I believe that's the issue.

The issue is actually caused by a recent update to async-substrate-interface which slowed down things too much to work correctly. I have a fix up for it (opentensor/async-substrate-interface#56), and version 1.0.3 should be released shortly, which will allow for these to pass.

@ibraheem-opentensor ibraheem-opentensor merged commit 4c6a4b0 into opentensor:staging Feb 20, 2025
1 check passed
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