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
Feature/user-api #1697
Feature/user-api #1697
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Rutvikrj26! I've added a few comments inline. There are also a few styling issues that need fixing for the tests to pass. Please could you take a look?
Example output for http://localhost:8000/projects/api/v1/published
Example output for http://localhost:8000/projects/api/v1/published/1
|
I wonder if we should be using a look up table/dictionary to map variables from the models to the API data? (e.g. we could map "dua" on the model to "data_use_agreement" in the API data). This would (1) allow us to choose more meaningful names than those on the models and (2) protect the API from changes to the models. |
-> slug/version instead of
Only output these fields:
I think "license" and "dua" would be nice to include in some way, but not as integers as they are. |
I was thinking the same thing. Fixing some of the Enum types that we use for the class models would probably be a first step for this. The Django enum type (see: https://docs.djangoproject.com/en/4.1/ref/models/fields/#enumeration-types) supports human readable names, which is a nice feature. For some reason, physionet-build/physionet-django/project/modelcomponents/access.py Lines 16 to 30 in 141d2a9
|
35e7000
to
f30408b
Compare
Current API Calls & their Responses :
Call : http://127.0.0.1:8000/projects/api/v1/published/demowave/1.0.0
|
Final API Calls : Call : http://127.0.0.1:8000/projects/api/v1/published
Call : http://127.0.0.1:8000/projects/api/v1/published/demowave/1.0.0
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Rutvikrj26, I added some minor comments inline, but overall this looks good to me.
One issue is that there is no way to identify the versions using the API. http://127.0.0.1:8000/projects/api/v1/published does not list project version.
This means that there is no clear way for someone to discover the project path.
For example, how would someone find the following URL: http://127.0.0.1:8000/projects/api/v1/published/demowave/1.0.0
I still don't know what the application is here. Can you give a specific, concrete example? What is it you're trying to do? Why are the existing views in export/ not sufficient for that purpose? |
@bemoody |
@Rutvikrj26 we caught up on this pull request today. The outcome was that we would like to combine your changes with the export app: https://github.com/MIT-LCP/physionet-build/tree/dev/physionet-django/export Please could you move your changes into the export app using whichever aspects of each implementation you prefer? In general I would suggest defaulting to your code. All of the current views at https://github.com/MIT-LCP/physionet-build/blob/dev/physionet-django/export/views.py can be removed. |
@bemoody we have had several email requests from people who would like access to an API that allows them to search for project content using an API. One example is a group who maintain an index of datasets that might be of interest to their research staff. They would like to be able to populate their search index using an API such as the one implemented here. Our goal is to introduce an API with very simple functionality and then build it out as we have a clearer understanding of the different uses cases. |
This looks good to me, except that Benjamin has found a dependency on the def get_dbs():
"""
Get a list of all the PhysioNet databases available.
Parameters
----------
N/A
Returns
-------
dbs : list
All of the databases currently available for analysis.
Examples
--------
>>> dbs = wfdb.get_dbs()
>>> dbs
[
['aami-ec13', 'ANSI/AAMI EC13 Test Waveforms'],
['adfecgdb', 'Abdominal and Direct Fetal ECG Database'],
...
['wrist', 'Wrist PPG During Exercise']
]
"""
with _url.openurl("https://physionet.org/rest/database-list/", "rb") as f:
content = f.read()
dbs = json.loads(content)
dbs = [[d["slug"], d["title"]] for d in dbs]
dbs.sort()
return dbs Is it possible to reinstate this view and URL temporarily? We can then work towards replacing it in the wfdb-python package and subsequent deprecation. |
Sure! Note : Current API call :
I am adding the temporary Database list with the same original |
Don't remove TEST_DEFAULTS from physionet-django/export/urls.py. Instead, define some useful test parameters, e.g.
|
(It might be better to rename the URL parameter 'slug' to 'project_slug' for clarity and consistency with the rest of the site.) |
Thanks @Rutvikrj26, overall this looks good to me. I think there are several improvements that we should make, but this gives us a good starting point. e.g.
|
This Pull request introduces an API for Published Projects.
There are currently two paths :
/projects/api/v1/published
: Provides a list of all the published projects (id, title, abstract)/projects/api/v1/published/<project-slug>/<project-version>
: Provides all MetaData for that specific Published Project.The API is available without Authentication.
Pending :