dandi / dandi-schema

Schemata for DANDI archive project
Apache License 2.0
5 stars 8 forks source link

Set the `$schema` key with the schema dialect #236

Closed candleindark closed 2 months ago

candleindark commented 2 months ago

This PR closes #216.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.83%. Comparing base (57b503a) to head (5841e4e). Report is 12 commits behind head on master.

:exclamation: Current head 5841e4e differs from pull request most recent head 862f25f. Consider uploading reports for the commit 862f25f to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #236 +/- ## ========================================== - Coverage 97.58% 91.83% -5.76% ========================================== Files 16 16 Lines 1701 1727 +26 ========================================== - Hits 1660 1586 -74 - Misses 41 141 +100 ``` | [Flag](https://app.codecov.io/gh/dandi/dandi-schema/pull/236/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dandi/dandi-schema/pull/236/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | `91.83% <100.00%> (-5.76%)` | :arrow_down: | 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.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

candleindark commented 2 months ago

@yarikoptic @satra Adding the $schema key will change the resulting schemas. Should we up the version to 0.6.8?

yarikoptic commented 2 months ago

I think it is indeed would be desired. Please add an explanation as well as you did in 09e50e17db87321ccfad6c6c8fd5c40a3814ae37

yarikoptic commented 2 months ago

although may be we should just set it upon adding a PR which has release label -- that is when our "Test that old schemata are not modified" kicks in as well?

candleindark commented 2 months ago

@yarikoptic I updated the DANDI_SCHEMA_VERSION to "0.6.8". I am not sure if this PR should have the release label. I don't see it as a major change though it alters the generated schemas.

yarikoptic commented 2 months ago

let's drop last commit and proceed without releasing for now

git reset --hard HEAD^
git push -f
candleindark commented 2 months ago

@yarikoptic I trimmed this PR down to the only essential commit. However, for some reason, some of the cli tests failed at the following.

FAILED tests/test_download.py::test_download_newest_version - ValueError: Dandiset 000077 is Invalid: {
    "asset_validation_errors": [],
    "version_validation_errors": [
        {
            "field": "contributor",
            "message": "Value error, Contact person must have an email address."
        },
        {
            "field": "contributor",
            "message": "Input should be 'Organization'"
        }
    ]
}

That error is only included in https://github.com/dandi/dandi-schema/commit/dc15401c895f12186491247533587313a88c010f which is not yet merged with this PR. How does this come about?

The other error is

  Version 3.8 was not found in the local cache
  Error: The version '3.8' with architecture 'arm64' was not found for macOS 14.4.1.
  The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

It seems that Python 3.8 is no longer available for the latests macOS version.

yarikoptic commented 2 months ago

pull_request is tested after merging master which has https://github.com/dandi/dandi-schema/pull/235 merged now, and apparently I didn't mention that dandi-cli tests were failing there whenever I clicked "Merge" . See e.g. https://github.com/dandi/dandi-schema/actions/runs/8696464081/job/23849776507 so we need to fix them in dandi-cli now. Filed

yarikoptic commented 2 months ago

The other error is

  Version 3.8 was not found in the local cache
  Error: The version '3.8' with architecture 'arm64' was not found for macOS 14.4.1.
  The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

it seems also for 3.9 . But note that it is arm64 -- I wonder if they just switched...

looking at our dump of logs we keep on drogon:

dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2024/04$ ls -ld */github/cron/*/*/test.yml/*/*macos*3.8*
...
drwxr-sr-x 1 dandi dandi   436 Apr 16 02:20 '16/github/cron/20240416T060243/d46af61/test.yml/2035-success/test (macos-latest, 3.8)'
-rw-r--r-- 1 dandi dandi 49462 Apr 17 02:20 '17/github/cron/20240417T060248/d46af61/test.yml/2036-success/10_test (macos-latest, 3.8).txt'
drwxr-sr-x 1 dandi dandi   436 Apr 17 02:20 '17/github/cron/20240417T060248/d46af61/test.yml/2036-success/test (macos-latest, 3.8)'
-rw-r--r-- 1 dandi dandi 55340 Apr 18 02:20 '18/github/cron/20240418T060235/dc15401/test.yml/2039-failed/10_test (macos-latest, 3.8).txt'
drwxr-sr-x 1 dandi dandi   314 Apr 18 02:20 '18/github/cron/20240418T060235/dc15401/test.yml/2039-failed/test (macos-latest, 3.8)'
...

so it flipped around 18th... but the fail there is not what you quoted, it is again about this new aspect of requiring email for contact person including the last one from today:

2024-04-23T06:04:51.7209590Z ____________________________ test_datacite[000008] _____________________________
2024-04-23T06:04:51.7218140Z dandischema/tests/test_datacite.py:157: in test_datacite
2024-04-23T06:04:51.7219320Z     meta = PublishedDandiset(**meta_js)
2024-04-23T06:04:51.7221660Z E   pydantic_core._pydantic_core.ValidationError: 5 validation errors for PublishedDandiset
2024-04-23T06:04:51.7224590Z E   contributor.0.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7226380Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'schemaKey': 'Person', '...ncludeInCitation': True}, input_type=dict]
2024-04-23T06:04:51.7227760Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7229220Z E   contributor.0.function-after[ensure_contact_person_has_email(), Organization].schemaKey
2024-04-23T06:04:51.7230460Z E     Input should be 'Organization' [type=literal_error, input_value='Person', input_type=str]
2024-04-23T06:04:51.7231510Z E       For further information visit https://errors.pydantic.dev/2.7/v/literal_error
2024-04-23T06:04:51.7232650Z E   contributor.16.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7234200Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'schemaKey': 'Person', '...ncludeInCitation': True}, input_type=dict]
2024-04-23T06:04:51.7235530Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7236690Z E   contributor.16.function-after[ensure_contact_person_has_email(), Organization].schemaKey
2024-04-23T06:04:51.7237910Z E     Input should be 'Organization' [type=literal_error, input_value='Person', input_type=str]
2024-04-23T06:04:51.7239290Z E       For further information visit https://errors.pydantic.dev/2.7/v/literal_error
2024-04-23T06:04:51.7240640Z E   contributor.16.function-after[ensure_contact_person_has_email(), Organization].identifier
2024-04-23T06:04:51.7242170Z E     String should match pattern '^https://ror.org/[a-z0-9]+$' [type=string_pattern_mismatch, input_value='0000-0002-4305-6376', input_type=str]
2024-04-23T06:04:51.7243730Z E       For further information visit https://errors.pydantic.dev/2.7/v/string_pattern_mismatch
2024-04-23T06:04:51.7244900Z __________ test_dandimeta_datacite[additional_meta1-datacite_checks1] __________
2024-04-23T06:04:51.7245780Z dandischema/tests/test_datacite.py:385: in test_dandimeta_datacite
2024-04-23T06:04:51.7246490Z     datacite = to_datacite(metadata_basic)
2024-04-23T06:04:51.7247030Z dandischema/datacite.py:66: in to_datacite
2024-04-23T06:04:51.7247520Z     meta = PublishedDandiset(**meta)
2024-04-23T06:04:51.7248300Z E   pydantic_core._pydantic_core.ValidationError: 2 validation errors for PublishedDandiset
2024-04-23T06:04:51.7249380Z E   contributor.0.function-after[ensure_contact_person_has_email(), Person]
2024-04-23T06:04:51.7250830Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'name': 'A_last, A_first...'dcite:ContactPerson'>]}, input_type=dict]
2024-04-23T06:04:51.7252110Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7253120Z E   contributor.0.function-after[ensure_contact_person_has_email(), Organization]
2024-04-23T06:04:51.7254590Z E     Value error, Contact person must have an email address. [type=value_error, input_value={'name': 'A_last, A_first...'dcite:ContactPerson'>]}, input_type=dict]
2024-04-23T06:04:51.7255860Z E       For further information visit https://errors.pydantic.dev/2.7/v/value_error
2024-04-23T06:04:51.7256840Z __________ test_dandimeta_datacite[additional_meta2-datacite_checks2] __________
...

which adds to the insult of me missing all those fails while merging... Do you think you could look into them asap or should I?

re arm64, I see it emerged only today in this PR:

23/github/pr/236/862f25f/test-dandi-cli.yml/1447-failed/4_test-dandi-cli (macos-latest, 3.8, master, normal).txt:2024-04-23T18:53:10.1224310Z Image: macos-14-arm64

and that flipped today -- before that it was macos-12:

dandi@drogon:/mnt/backup/dandi/tinuous-logs/dandischema/2024/04$ git grep 'Image:'  -- '*/test.yml/*/10_test (macos-latest, 3.8).txt'
01/github/cron/20240401T060245/57b503a/test.yml/2006-success/10_test (macos-latest, 3.8).txt:2024-04-01T06:02:53.7957430Z Image: macos-12
...
22/github/cron/20240422T060246/dc15401/test.yml/2043-failed/10_test (macos-latest, 3.8).txt:2024-04-22T06:02:54.5086340Z Image: macos-12
23/github/cron/20240423T060233/dc15401/test.yml/2044-failed/10_test (macos-latest, 3.8).txt:2024-04-23T06:03:55.7784620Z Image: macos-12
23/github/pr/236/862f25f/test.yml/2048-failed/10_test (macos-latest, 3.8).txt:2024-04-23T18:52:54.4118480Z Image: macos-14-arm64
23/github/pr/236/862f25f/test.yml/2050-failed/10_test (macos-latest, 3.8).txt:2024-04-23T18:59:26.5157860Z Image: macos-14-arm64

let's see if

candleindark commented 2 months ago

Those errors are very strange, but I think will try to resolve https://github.com/dandi/dandi-cli/issues/1430 first.