Skip to content

fix: converting to staticmethod some tests in AstraDocumentStore#2928

Merged
davidsbatista merged 13 commits intomainfrom
fix/static-methods-Astra
Mar 13, 2026
Merged

fix: converting to staticmethod some tests in AstraDocumentStore#2928
davidsbatista merged 13 commits intomainfrom
fix/static-methods-Astra

Conversation

@davidsbatista
Copy link
Copy Markdown
Contributor

Checklist

@davidsbatista davidsbatista requested a review from a team as a code owner March 6, 2026 15:21
@davidsbatista davidsbatista requested review from julian-risch and removed request for a team March 6, 2026 15:21
@davidsbatista
Copy link
Copy Markdown
Contributor Author

when you have time @julian-risch have look on this one

Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

Looks good to me!
My only comment is that ChromaDocumentStore is the only DocumentStore so far where we have this as staticmethod and we should aim for consistency in the codebase: https://github.com/search?q=repo%3Adeepset-ai%2Fhaystack-core-integrations%20assert_documents_are_equal&type=code

@davidsbatista
Copy link
Copy Markdown
Contributor Author

This is a pattern I see often.

I think it has to do with IDE code completion that automatically assumes it's an instance method and always adds the self as the first param.

Not a top priority, but I always tend to correct those when I see them. I can adjust/correct the others as I make each doc store use the Mixin tests.

@davidsbatista davidsbatista merged commit d5a8d3e into main Mar 13, 2026
12 checks passed
@davidsbatista davidsbatista deleted the fix/static-methods-Astra branch March 13, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants