-
Notifications
You must be signed in to change notification settings - Fork 38
Refactoring inline imports and resolve circular dependencies #113
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
Changes from all commits
62e6aeb
1b85245
63f4ea9
815aeed
d3aef1a
c36657a
0638fe2
1c81c1d
afa55d2
d2864eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,25 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why change this file at all? I it has no inline imports.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ruff formats imports according to the PEP 8 import ordering rules, which generally means:
Ruff’s import sorter (ruff --fix, rule I001, internally inspired by isort) enforces: |
||
| # Licensed under the MIT License. | ||
|
|
||
| from collections.abc import AsyncGenerator, Iterator | ||
| import os | ||
| import tempfile | ||
| from collections.abc import AsyncGenerator, Iterator | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar here. |
||
| from typing import Any | ||
|
|
||
| import pytest | ||
| import pytest_asyncio | ||
|
|
||
| import tiktoken | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here. |
||
| from openai.types.create_embedding_response import CreateEmbeddingResponse, Usage | ||
| from openai.types.embedding import Embedding | ||
| import tiktoken | ||
|
|
||
| from typeagent.aitools import utils | ||
| from typeagent.aitools.embeddings import AsyncEmbeddingModel, TEST_MODEL_NAME | ||
| from typeagent.aitools.embeddings import TEST_MODEL_NAME, AsyncEmbeddingModel | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leave the ordering alone. Imported things go alphabetically. |
||
| from typeagent.aitools.vectorbase import TextEmbeddingIndexSettings | ||
| from typeagent.storage.memory.collections import ( | ||
| MemoryMessageCollection, | ||
| MemorySemanticRefCollection, | ||
| from typeagent.knowpro.convsettings import ( | ||
| ConversationSettings, | ||
| MessageTextIndexSettings, | ||
| RelatedTermIndexSettings, | ||
| ) | ||
| from typeagent.knowpro.convsettings import ConversationSettings | ||
| from typeagent.knowpro.interfaces import ( | ||
| DeletionInfo, | ||
| IConversation, | ||
|
|
@@ -30,18 +29,18 @@ | |
| ISemanticRefCollection, | ||
| IStorageProvider, | ||
| ITermToSemanticRefIndex, | ||
| SemanticRef, | ||
| ScoredSemanticRefOrdinal, | ||
| SemanticRef, | ||
| TextLocation, | ||
| ) | ||
| from typeagent.knowpro.kplib import KnowledgeResponse | ||
| from typeagent.knowpro.convsettings import ( | ||
| MessageTextIndexSettings, | ||
| RelatedTermIndexSettings, | ||
| ) | ||
| from typeagent.knowpro.secindex import ConversationSecondaryIndexes | ||
| from typeagent.storage.memory import MemoryStorageProvider | ||
| from typeagent.storage import SqliteStorageProvider | ||
| from typeagent.storage.memory import MemoryStorageProvider | ||
| from typeagent.storage.memory.collections import ( | ||
| MemoryMessageCollection, | ||
| MemorySemanticRefCollection, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |
|
|
||
| import pytest | ||
|
|
||
| from typeagent.aitools.embeddings import AsyncEmbeddingModel, TEST_MODEL_NAME | ||
| from typeagent.aitools.embeddings import TEST_MODEL_NAME, AsyncEmbeddingModel | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again on both count -- why change this file, and why reorder the items here. |
||
| from typeagent.knowpro.convsettings import ConversationSettings | ||
| from typeagent.storage.sqlite.provider import SqliteStorageProvider | ||
| from typeagent.transcripts.transcript import ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| # Licensed under the MIT License. | ||
|
|
||
| import time | ||
|
|
||
| import pytest | ||
| from pytest_mock import MockerFixture | ||
|
|
||
|
|
||
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.
The way I like my imports sorted (by full module name), 'time' follows 'pathlib'.