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 some noqa: ARG rules escapes #84

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Remove some noqa: ARG rules escapes #84

merged 2 commits into from
Sep 30, 2024

Conversation

cbornet
Copy link
Collaborator

@cbornet cbornet commented Sep 30, 2024

Fix #83

@cbornet cbornet changed the title Remove some rules escapes Remove some noqa: ARG rules escapes Sep 30, 2024
@@ -489,15 +486,14 @@ async def test_astradb_vectorstore_indexing_custom_async(
def test_astradb_vectorstore_vectorize_headers_precedence_stringheader(
self,
astra_db_credentials: AstraDBCredentials,
collection_vz: Collection, # noqa: ARG002
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Injecting collection_vz had no use here ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I think you spotted a bug in testing (both here and in the subsequent test_astradb_vectorstore_vectorize_headers_precedence_headerprovider). Well done!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pushed a fix.

Copy link
Collaborator

@hemidactylus hemidactylus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for cleaning the noqas and also replacing the constants with <collection>.name !

@hemidactylus hemidactylus merged commit fc1bc6c into main Sep 30, 2024
13 checks passed
@hemidactylus hemidactylus deleted the fix-ruff-arg branch September 30, 2024 13:22
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.

Use pytest.mark.usefixtures
2 participants