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.

Sign up for GitHub

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

Jump to bottom

Add B2Share support #88 #89

Merged
merged 28 commits into from
Sep 30, 2024
Merged

Add B2Share support #88 #89

merged 28 commits into from
Sep 30, 2024

Conversation

micafer
Copy link
Contributor

@micafer micafer commented Sep 24, 2024

No description provided.

micafer and others added 19 commits May 30, 2024 14:06
@micafer
Fix J535D165#75
820bb3a
@micafer
Merge pull request #1 from micafer/fix_75
04d5468 
Fix J535D165#75
@micafer
Update README.md
07db0ac
@micafer
Enable to remove the last slash in the osf url
3243ea4
@micafer
Add support for providers in OSF: J535D165#69
06bda68
@micafer
Merge pull request #2 from micafer/osf_providers
442bca3 
Add support for providers in OSF: J535D165#69
@micafer
Fix style
b0d0c28
@micafer
Add support to filter files
f842007
@micafer
Add support to filter files
dd99815
@micafer
Merge pull request #3 from micafer/filter
d9ad19b 
Filter
@micafer
Add support to SeaNoe
6c00ebd
@micafer
Implements data.europa.eu support
31906b5
@micafer
merge upstream
257afa4
@micafer
Merge branch 'seanoe'
4a8c560
@micafer
merge data_europa_eu
16d2518
@micafer
Minor changes
62698ef
@micafer
Add B2Share support J535D165#88
659e2d3
@micafer
Merge branch 'main' into b2share
21e89a1
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
9be54fd 
for more information, see https://pre-commit.ci
@J535D165
Copy link
Owner

It would be very cool to add support for this!

Can you rebase?

@J535D165 J535D165 added enhancement New feature or request new repository Add support for a new repository labels Sep 29, 2024
@J535D165
Copy link
Owner

It might also be nice to split the filter-files extension from the PR so that we can have a closer look at the design. It looks promising.

@micafer
Copy link
Contributor Author

micafer commented Sep 30, 2024

Code cleaned.

J535D165
J535D165 reviewed Sep 30, 2024
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
J535D165
J535D165 reviewed Sep 30, 2024
View reviewed changes
datahugger/base.py Outdated Show resolved Hide resolved
@J535D165 J535D165 linked an issue Sep 30, 2024 that may be closed by this pull request
Support B2Share repository #88
Closed
J535D165
J535D165 reviewed Sep 30, 2024
View reviewed changes
datahugger/services.py
@@ -320,7 +320,7 @@ class MendeleyDataset(DatasetDownloader):
class OSFDataset(DatasetDownloader):
"""Downloader for OSF repository."""

REGEXP_ID = r"osf\.io\/(?P<record_id>.*)/"
REGEXP_ID = r"osf\.io\/(?P<record_id>[^\/]*)\/{0,1}"
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting catch. I'm fine with keeping this in this PR.

@J535D165
Copy link
Owner

Apparently, the test fails because I removed the raise_for_status d8a4a49. I'm a bit confused.

J535D165
J535D165 reviewed Sep 30, 2024
View reviewed changes
datahugger/services.py Outdated Show resolved Hide resolved
@J535D165
Copy link
Owner

Now it passes. Did they change something to their API since the commit of @micafer 5 hours ago...?

Nevertheless, thanks a lot, @micafer, for another cool contribution!

@J535D165 J535D165 merged commit 66d71ab into J535D165:main Sep 30, 2024
19 checks passed
@micafer
Copy link
Contributor Author

micafer commented Oct 1, 2024

Now it passes. Did they change something to their API since the commit of @micafer 5 hours ago...?

It should not have changed.

@micafer
Copy link
Contributor Author

micafer commented Oct 1, 2024

Apparently, the test fails because I removed the raise_for_status d8a4a49. I'm a bit confused.

It should not affect the tests.
I added to improve the error message shown to the user in case of HTTP error. Without it the HTTP error is hidden and the error message shown is a bit confusing,

@J535D165
Copy link
Owner

J535D165 commented Oct 1, 2024

It works now! I tested it on the benchmark dataset. Everything looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J535D165 J535D165 J535D165 left review comments

Assignees
No one assigned
Labels
enhancement New feature or request new repository Add support for a new repository
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support B2Share repository
2 participants
@micafer @J535D165