fix: clean up optional components of the schema #52
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
didn't look inside yet but tests (besides dandi-cli) seems to be not happy. |
Codecov Report
@@ Coverage Diff @@
## master #52 +/- ##
==========================================
+ Coverage 95.87% 95.91% +0.04%
==========================================
Files 11 11
Lines 1042 1053 +11
==========================================
+ Hits 999 1010 +11
Misses 43 43
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@yarikoptic - should be ok now |
yarikoptic
reviewed
Jul 8, 2021
| @@ -1007,15 +1019,15 @@ class Asset(BareAsset): | |||
| # all of the following are set by server | |||
| id: str = Field(readOnly=True, description="Uniform resource identifier") | |||
| identifier: UUID4 = Field(readOnly=True, nskey="schema") | |||
| contentUrl: List[HttpUrl] = Field(None, readOnly=True, nskey="schema") | |||
| contentUrl: List[HttpUrl] = Field(readOnly=True, nskey="schema") | |||
There was a problem hiding this comment.
hm, this is in Asset but it seems somehow to cause dandi-cli to fail validation now:
2021-07-07T21:25:05.3669152Z _____________________________ test_metadata2asset ______________________________
2021-07-07T21:25:05.3669469Z
2021-07-07T21:25:05.3669855Z def test_metadata2asset():
2021-07-07T21:25:05.3670409Z data = metadata2asset(
2021-07-07T21:25:05.3670793Z {
2021-07-07T21:25:05.3671161Z "contentSize": 69105,
2021-07-07T21:25:05.3671868Z "digest": "e455839e5ab2fa659861f58a423fd17f-1",
2021-07-07T21:25:05.3672429Z "digest_type": "dandi_etag",
2021-07-07T21:25:05.3673088Z "encodingFormat": "application/x-nwb",
2021-07-07T21:25:05.3673762Z "experiment_description": "Experiment Description",
2021-07-07T21:25:05.3674414Z "experimenter": "Joe Q. Experimenter",
2021-07-07T21:25:05.3675309Z "id": "dandiasset:0b0a1a0b-e3ea-4cf6-be94-e02c830d54be",
2021-07-07T21:25:05.3676010Z "institution": "University College",
2021-07-07T21:25:05.3676745Z "keywords": ["test", "sample", "example", "test-case"],
2021-07-07T21:25:05.3677309Z "lab": "Retriever Laboratory",
2021-07-07T21:25:05.3758449Z "related_publications": "A Brief History of Test Cases",
2021-07-07T21:25:05.3759216Z "session_description": "Some test data",
2021-07-07T21:25:05.3759745Z "session_id": "XYZ789",
2021-07-07T21:25:05.3760632Z "session_start_time": "2020-08-31T15:58:28-04:00",
2021-07-07T21:25:05.3761091Z "age": "23 days",
2021-07-07T21:25:05.3761715Z "date_of_birth": "2020-03-14T12:34:56-04:00",
2021-07-07T21:25:05.3762186Z "genotype": "Typical",
2021-07-07T21:25:05.3762610Z "sex": "M",
2021-07-07T21:25:05.3763021Z "species": "human",
2021-07-07T21:25:05.3763463Z "subject_id": "a1b2c3",
2021-07-07T21:25:05.3763905Z "cell_id": "cell01",
2021-07-07T21:25:05.3764338Z "slice_id": "slice02",
2021-07-07T21:25:05.3764837Z "tissue_sample_id": "tissue03",
2021-07-07T21:25:05.3765322Z "probe_ids": "probe04",
2021-07-07T21:25:05.3765828Z "number_of_electrodes": 42,
2021-07-07T21:25:05.3766284Z "number_of_units": 6,
2021-07-07T21:25:05.3766713Z "nwb_version": "2.2.5",
2021-07-07T21:25:05.3767108Z "nd_types": [
2021-07-07T21:25:05.3767506Z "Device (2)",
2021-07-07T21:25:05.3767941Z "DynamicTable",
2021-07-07T21:25:05.3768435Z "ElectricalSeries",
2021-07-07T21:25:05.3768969Z "ElectrodeGroup",
2021-07-07T21:25:05.3769413Z "Subject",
2021-07-07T21:25:05.3769772Z ],
2021-07-07T21:25:05.3770146Z "path": "/test/path",
2021-07-07T21:25:05.3770529Z }
2021-07-07T21:25:05.3770830Z )
2021-07-07T21:25:05.3771375Z with (METADATA_DIR / "metadata2asset.json").open() as fp:
2021-07-07T21:25:05.3771989Z data_as_dict = json.load(fp)
2021-07-07T21:25:05.3772581Z data_as_dict["schemaVersion"] = DANDI_SCHEMA_VERSION
2021-07-07T21:25:05.3773522Z assert data == BareAssetMeta(**data_as_dict)
2021-07-07T21:25:05.3774100Z bare_dict = deepcopy(data_as_dict)
2021-07-07T21:25:05.3774636Z assert data.json_dict() == bare_dict
2021-07-07T21:25:05.3775586Z data_as_dict["identifier"] = "0b0a1a0b-e3ea-4cf6-be94-e02c830d54be"
2021-07-07T21:25:05.3776321Z > validate(data_as_dict)
2021-07-07T21:25:05.3776627Z
2021-07-07T21:25:05.3777504Z /opt/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/dandi/tests/test_metadata.py:177:
2021-07-07T21:25:05.3778259Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2021-07-07T21:25:05.3779238Z /opt/hostedtoolcache/Python/3.7.10/x64/lib/python3.7/site-packages/dandischema/metadata.py:155: in validate
2021-07-07T21:25:05.3780006Z klass(**obj)
2021-07-07T21:25:05.3780388Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2021-07-07T21:25:05.3780667Z
2021-07-07T21:25:05.3780967Z > ???
2021-07-07T21:25:05.3781722Z E pydantic.error_wrappers.ValidationError: 1 validation error for Asset
2021-07-07T21:25:05.3782495Z E contentUrl
2021-07-07T21:25:05.3783023Z E field required (type=value_error.missing)
2021-07-07T21:25:05.3783417Z
2021-07-07T21:25:05.3783881Z pydantic/main.py:406: ValidationError
and that is ok in recent master build
(dandisets) dandi@drogon:~/cronlib/tinuous-logs/dandischema/2021/07/07/github/cron/20210707T061237/4f6e708/test-dandi-cli.yml/11-failed$ grep test_metadata2asset 17_test-dandi-cli\ \(ubuntu-18.04\,\ 3.7\,\ dandi-devel\,\ master\).txt
2021-07-07T06:15:48.9393505Z tests/test_metadata.py::test_metadata2asset PASSED
2021-07-07T06:15:48.9426456Z tests/test_metadata.py::test_metadata2asset_simple1 PASSEDThere was a problem hiding this comment.
validate does so against Asset even for BareAsset since the schemaKey is set to Asset:
thus contentUrl would be required.
There was a problem hiding this comment.
so what would we do then to mitigate for BareAsset's -- provide file:// urls?
There was a problem hiding this comment.
anyways, since we boosted it to 0.5.0, and if that is how it is to be, we can proceed here and will just need to fix up dandi-cli to account for this
There was a problem hiding this comment.
we might need to reconsider the schemaKey "casting" we do, seems to start to bite ;)
yarikoptic
reviewed
Jul 8, 2021
Co-authored-by: Yaroslav Halchenko <[email protected]>
|
you would want this to be released @satra (do not see a tag) or we better absorb more changes before the release? |
|
i wanted to wait to see if there were other changes to be made to the schema before tagging for release. i want to slowly start consolidating schema changes as much as possible. |
|
ok, then let's proceed with a merge without release |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR cleans up different elements of the schema that were a mismatch between expectations and validation mostly around the use of None.