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

max_results: float vs. int #123

Closed
radoshi opened this issue Oct 2, 2023 · 4 comments
Closed

max_results: float vs. int #123

radoshi opened this issue Oct 2, 2023 · 4 comments
Labels
question Questions about how to use this package.

Comments

@radoshi
Copy link

radoshi commented Oct 2, 2023

Hello! Thanks for the neat library.

The max_results parameter on the Search object makes it appear that you want a float (to deal with inf). However, passing in an actual float, results in an error.

Here it is, exercised as a test:
https://gist.github.com/radoshi/a43862d438dc0d838d10b89ff0e0a564

I'm curious why you chose the float(inf) approach vs just hardcoding to max_results=300000
https://gist.github.com/radoshi/a9e3d0d9b947459c4ff96a46f6e35d55

Easy PR, happy to open it.
Thanks again for the lib.

@radoshi radoshi added the question Questions about how to use this package. label Oct 2, 2023
@lukasschwab
Copy link
Owner

lukasschwab commented Oct 2, 2023

You're right, it should be an integer. I think I was looking for some MAX_INT-ish constant and couldn't find one; ints are apparently unbounded in Python 3.

I'd gladly merge a PR!

The library still needs an unbounded default. Treating negative-max_results queries as unbounded queries is probably the most conventional.

arxiv.Search(
  query: str = "test",
  max_results: int = -1 # Equivalent to the old float("inf") behavior.
)

Good ticket — thanks for raising it,
Lukas

@radoshi
Copy link
Author

radoshi commented Oct 2, 2023

Cool. I'll get something to you. I think -1 will create a different issue, likely in this line:
page_size = min(self.page_size, search.max_results - offset)

We can continue to the conversation on the PR.

@radoshi
Copy link
Author

radoshi commented Oct 4, 2023

PR: #124
Ready for your review.

@lukasschwab
Copy link
Owner

Closed by #138.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Questions about how to use this package.
Projects
None yet
Development

No branches or pull requests

2 participants