Skip to content

Restructure / implement more functionality#1

Open
mariogiov wants to merge 11 commits intonh13:masterfrom
mariogiov:master
Open

Restructure / implement more functionality#1
mariogiov wants to merge 11 commits intonh13:masterfrom
mariogiov:master

Conversation

@mariogiov
Copy link
Copy Markdown

Hi Nils!

I was looking for a good wrapper for downloading files from BaseSpace and I found your scripts! Great initiative. I think a good set of wrappers for additional functionality would be great. I rewrote this script pretty heavily (mostly just to be a bit more Pythonic) while keeping the general flow you had. I think the result is quite tidy but there are still some more features I'd like to add and some more restructuring as well. If you'd rather skip this PR that's fine but I think collaborations are always a good thing (and I love the ascii art!).

Best,
Mario

Comment thread src/scripts/download_files.py Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

what if we want a specific sample within a project?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, I think the more restrictive use-case as you describe it is the better one.

@nh13
Copy link
Copy Markdown
Owner

nh13 commented Sep 17, 2014

Unfortunately, this breaks backwards compatibility with a common use case. If I have a project that is shared with me, it is not listed when I simply specify the project id using the -p option. It works if I update the line:

    basespace_projects = myAPI.getProjectByUser()

to:

    from BaseSpacePy.model.QueryParameters import QueryParameters as qp
    basespace_projects = myAPI.getProjectByUser(qp({'Limit' : 1024}))

Perhaps there is some default limit? Anyhow, looks good so far but the above needs to be fixed and I can continue reviewing.

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

Successfully merging this pull request may close these issues.

2 participants