dandi / dandi-schema

Schemata for DANDI archive project
Apache License 2.0
7 stars 10 forks source link

added funderidtype and support for funder and sponsor roles to datacite metadata #167

Closed satra closed 1 year ago

satra commented 1 year ago

closes #168

according to @djarecka's test the current release mints a doi for the failing metadata schema on dandiarchive, so unsure what the exact issue was. nonetheless this update should be used moving forward since people use funder/sponsor synonymously in dandiset metadata.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 88.88% and no project coverage change.

Comparison is base (845cb17) 97.71% compared to head (0f5c77a) 97.72%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #167 +/- ## ======================================= Coverage 97.71% 97.72% ======================================= Files 17 17 Lines 1751 1758 +7 ======================================= + Hits 1711 1718 +7 Misses 40 40 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `97.72% <88.88%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/dandi/dandi-schema/pull/167?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | Coverage Δ | | |---|---|---| | [dandischema/tests/test\_datacite.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/167?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvdGVzdHMvdGVzdF9kYXRhY2l0ZS5weQ==) | `100.00% <ø> (ø)` | | | [dandischema/datacite.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/167?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvZGF0YWNpdGUucHk=) | `95.72% <88.88%> (+0.27%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

djarecka commented 1 year ago

lgtm

yarikoptic commented 1 year ago

according to @djarecka's test the current release mints a doi for the failing metadata schema on dandiarchive,

if we got to the bottom of it correctly during today's meeting, @djarecka didn't try to actually feed it to test datacite instance. Is the test where you added test case feeding it to it via https://github.com/dandi/dandi-schema/blob/HEAD/dandischema/tests/test_datacite.py#L17 ? would it fail if you remove the code you have added? without confirming the issue we would be not quite sure we're solving it fully.

djarecka commented 1 year ago

yes, looks like the error was not from to_datacite. During today meeting I was testing, but either I did something wrong, or I was getting inconsistent results... will update on slack

djarecka commented 1 year ago

I think my inconsistent results could be because clean_doi doesn't work as expected...

Any idea why I get *** requests.exceptions.HTTPError: 405 Client Error: Method Not Allowed for url: https://api.test.datacite.org/dois/10.80507/dandi.000333/0.0.0

satra commented 1 year ago

did you call this function: https://github.com/dandi/dandi-schema/blob/4e35300ace2868831372ab958bc64852dc8582f5/dandischema/tests/test_datacite.py#L17 ?

if so the doi has already been deleted because of the last step.

djarecka commented 1 year ago

but you can check that https://api.test.datacite.org/dois/10.80507/dandi.000333/0.0.0 exists

satra commented 1 year ago

i don't know what's going on, but have you tried running the other function so that it posts and deletes? that should tell you it's working or not.

djarecka commented 1 year ago

usually it works... but have no idea what is wrong with my small test, will debug a bit longer...

djarecka commented 1 year ago

it looks the method clean_doi doesn't work after running to_datacite with publish=True (additional attributes["event"] = "publish" is added). Do you understand why?

satra commented 1 year ago

i'm not sure i'm following, is there some script you can DM me to test?

djarecka commented 1 year ago

Just to update here:

satra commented 1 year ago

thanks @djarecka - did you try it with the specific metadata that failed on dandiarchive with the current released code (i.e. that there was an issue before) and this branch (now it works)?

djarecka commented 1 year ago

I used the link for 458 that you pointed me to on Friday.

Sorry, forgot to add that I don't have any issue with master, so still seems like I'm not recreating the initial problem

satra commented 1 year ago

@mvandenburgh - is there a way you can try to retrigger the doi generation on the server side for that dandiset that did not get a doi?

mvandenburgh commented 1 year ago

@mvandenburgh - is there a way you can try to retrigger the doi generation on the server side for that dandiset that did not get a doi?

Yes. Should I retrigger the doi generation with the code in this PR?

satra commented 1 year ago

@mvandenburgh - i would retest with what's on the server itself, since @djarecka can't replicate the issue with current main branch of this repo (without this PR).

yarikoptic commented 1 year ago

I wonder if worth

djarecka commented 1 year ago

@mvandenburgh - could you please try to post the DOI with this branch? I've tested it in test https://api.test.datacite.org/dois with publish=True.

During the meeting on Monday I mentioned that api.test.datacite doesn't behave as api.datacite, but after checking carefully, that was my mistake, so hopefully this will solve the problem also with api.datacite

djarecka commented 1 year ago

@jwodder - do you know why dandi-cli are failing, not sure if the error comes from these changes...

jwodder commented 1 year ago

@djarecka See https://github.com/dandi/dandi-cli/pull/1243#issuecomment-1554480467.

djarecka commented 1 year ago

@mvandenburgh just tested and he was able to create the proper doi with this branch https://doi.datacite.org/dois/10.48324%2Fdandi.000458%2F0.230317.0039