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

Download multiple files from Pennsieve #12

Closed
Kayvv opened this issue Apr 20, 2023 · 5 comments
Closed

Download multiple files from Pennsieve #12

Kayvv opened this issue Apr 20, 2023 · 5 comments

Comments

@Kayvv
Copy link
Contributor

Kayvv commented Apr 20, 2023

Download file function in pennsieve.py seems to take a list of dictionaries as input, but it shows and error when I try to input a list with more than one dictionary. Also can add test case for downloading multiple files in test_pennsieve.py.

@nickerso
Copy link
Member

@athril @jwagenaar - just wondering if you have any thoughts on this one?

@athril
Copy link
Collaborator

athril commented May 18, 2023

Hi @Kayvv, can you provide a reproducible example of your error?

@athril athril closed this as completed May 18, 2023
@athril athril reopened this May 18, 2023
@Kayvv
Copy link
Contributor Author

Kayvv commented May 18, 2023

Hi @athril,
Here's an example of this issue:

def test_download_files():
    file_list = [
        {'name': '5HT_cellDensity_5.json',
         'datasetId': 292,
         'datasetVersion': 1,
         'size': 534547,
         'fileType': 'Json',
         'packageType': 'Unsupported',
         'icon': 'JSON',
         'uri': 's3://prd-sparc-discover-use1/292/1/files/derivative/5-HT/5HT_cellDensity_5.json',
         'createdAt': None,
         'sourcePackageId': 'N:package:b3df91b7-bb85-4647-a106-02208ea07945'},
        {'name': '5HT_cellDensity_Layout1_view.json',
         'datasetId': 292,
         'datasetVersion': 1,
         'size': 316,
         'fileType': 'Json',
         'packageType': 'Unsupported',
         'icon': 'JSON',
         'uri': 's3://prd-sparc-discover-use1/292/1/files/derivative/5-HT/5HT_cellDensity_Layout1_view.json',
         'createdAt': None,
         'sourcePackageId': 'N:package:c0028722-74fc-4ec5-b5c6-5c3335f9d49a'}
    ]
    p = PennsieveService(connect=False)
    p.download_file(file_list=file_list)

When I run this test I get a TypeError:

E TypeError: expected str, bytes or os.PathLike object, not dict

However, it works fine if I run them individually like this:

def test_download_files():
    file_list = [
        {'name': '5HT_cellDensity_5.json',
         'datasetId': 292,
         'datasetVersion': 1,
         'size': 534547,
         'fileType': 'Json',
         'packageType': 'Unsupported',
         'icon': 'JSON',
         'uri': 's3://prd-sparc-discover-use1/292/1/files/derivative/5-HT/5HT_cellDensity_5.json',
         'createdAt': None,
         'sourcePackageId': 'N:package:b3df91b7-bb85-4647-a106-02208ea07945'}
    ]
    p = PennsieveService(connect=False)
    p.download_file(file_list=file_list)

    file_list = [
        {'name': '5HT_cellDensity_Layout1_view.json',
         'datasetId': 292,
         'datasetVersion': 1,
         'size': 316,
         'fileType': 'Json',
         'packageType': 'Unsupported',
         'icon': 'JSON',
         'uri': 's3://prd-sparc-discover-use1/292/1/files/derivative/5-HT/5HT_cellDensity_Layout1_view.json',
         'createdAt': None,
         'sourcePackageId': 'N:package:c0028722-74fc-4ec5-b5c6-5c3335f9d49a'}
    ]
    p.download_file(file_list=file_list)

PS: I got these two file dictionary by running
p.list_files(limit=2, dataset_id=292)

While I was writing the documentation for zinchelper, I found the same error also appearing in the tutorial.ipynb here in the 12th code block:

https://github.com/nih-sparc/sparc.client/blob/main/docs/tutorial.ipynb

@athril
Copy link
Collaborator

athril commented May 19, 2023

Thank you, @Kayvv, I was able to reproduce your error.
Basically the error you're getting is caused by not providing output_name parameter for the download_file() function.
Anyway, this function should be secured more to prevent similar errors in the future. I will submit a patch next week, possibly also a test to cover this use case.

@athril
Copy link
Collaborator

athril commented Jun 1, 2023

Closed with PR #16

@athril athril closed this as completed Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants