-
-
Notifications
You must be signed in to change notification settings - Fork 135
Implement ScienceDirectSearch
using the PUT
method
#396
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
base: master
Are you sure you want to change the base?
Implement ScienceDirectSearch
using the PUT
method
#396
Conversation
That's a same that the query string changes. I'm thinking about how to minimize the necessary changes on the user end and how to stay as close to the other classes as possible. My first question is whether the |
Here is the request schema from the documentation:
There are no mandatory fields. However if the query has to many results we get |
Do users use either |
There is no restriction, users can also use both. However, using for example From the documentation:
|
In this case I would suggest the following: Consistency is key, as is the requirement to use information for the cache file. Can you implement that please? |
I implemented the suggestion to pass sds = ScienceDirectSearch('"neural radiance fields" AND "3D rendering"', date='2024') Regarding the cache, we cannot only use the sds_1 = ScienceDirectSearch(title='Assessing LLMs in malicious code deobfuscation of real-world malware campaigns', date='2024')
sds_2 = ScienceDirectSearch(title='Sampling latent material-property information from LLM-derived embedding representations', date='2024')
sds_1._cache_file_path == sds_2._cache_file_path
Therefore I suggest to keep the current implementation and use the complete flattened query dictionary as name. |
Agreed, but let's use that only when there is no query. |
5b09b03
to
4c5e213
Compare
It will be a problem if we remove functionality from the current classes. They're in use already. And frankly I prefer more data even if it is retrieved in a non-recommended way than less data retrieved the right way. I would put the PR on hold until ScienceDirect removes the GET method altogether. |
With the
The {'authors': [{'order': 1, 'name': 'Constantinos Patsakis'},
{'order': 2, 'name': 'Fran Casino'},
{'order': 3, 'name': 'Nikolaos Lykousas'}]} The We could also keep the field names of the old method ( Finally, what I find most problematic of |
Ok, then we can keep it. But in any case, this requires a new major version. Not soon, though. |
PR for Issue #395. To implement the
PUT
method the whole search pipeline had to be extended. Summarised, the following changes were conducted:get_content
to use thePUT
methodsBase
. Unfortunately, the pagination logic is different to theScopusSearch
which usesGET
. Therefore I had to use a new conditional clause.Retrieve
class. Here were also some incompatibilities (query is a nested dictionary) which involved new conditional clauses.ScienceDirectSearch
class since the returned results where in a new format.