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

Not rebuilding SearchIndex every paper_search #512

Merged
merged 5 commits into from
Oct 1, 2024

Conversation

@jamesbraza jamesbraza added the enhancement New feature or request label Oct 1, 2024
@jamesbraza jamesbraza self-assigned this Oct 1, 2024
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 1, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Oct 1, 2024
Copy link
Contributor

@sidnarayanan sidnarayanan left a comment

Choose a reason for hiding this comment

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

Just one question

paperqa/agents/main.py Show resolved Hide resolved
@jamesbraza jamesbraza merged commit 63b6da5 into main Oct 1, 2024
5 checks passed
@jamesbraza jamesbraza deleted the splitting-index-build-get branch October 1, 2024 22:28
settings: Application settings.
build: Opt-out flag (default is True) to read the contents of the source paper
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO this logic is a bit complicated for users— what do you think about a “from_directory” class method on the SearchIndex class that you can use when you don’t need to build one? Then you can call that instead of adding an extra flag to this function. Logic looks good otherwise though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like you were approximately a few seconds too slow on the PR comment haha.

I didn't decompose get_directory_index in this PR because I was trying to be backwards compatible. I will make a downstream PR that does this then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
4 participants