dandi / dandi-schema

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

is it possible to make ContactPerson be invalid without email? #189

Closed yarikoptic closed 5 months ago

yarikoptic commented 1 year ago

We have https://github.com/dandisets/000108/blob/draft/dandiset.yaml where contact person is Lee , but he is listed without email.

- affiliation: []
  includeInCitation: true
  name: Kamentsky, Lee
  roleName:
  - dcite:ContactPerson
  schemaKey: Person
- affiliation: []
  email: smarx@mit.edu
  identifier: 0000-0002-8209-0059
  includeInCitation: true
  name: Marx, Slayton
  roleName:
  - dcite:DataManager
  - dcite:DataCurator
  - dcite:Investigation
  - dcite:ProjectMember
  - dcite:Researcher
  - dcite:Software
  - dcite:Supervision
  - dcite:Visualization
  schemaKey: Person

PS I now made @slaytonmarx to be the contact person since AFAIK Lee is no longer with the project. Please adjust if that is not true

yarikoptic commented 5 months ago

@satra @candleindark - how to express such a requirement in pydantic ?

candleindark commented 5 months ago

@satra @candleindark - how to express such a requirement in pydantic ?

Change https://github.com/dandi/dandi-schema/blob/57b503ade10140cec0a2c0cfab18e0dcba4977ee/dandischema/models.py#L823

to

email: Optional[EmailStr] = Field(..., json_schema_extra={"nskey": "schema"}) will do.

or

email: EmailStr = Field(..., json_schema_extra={"nskey": "schema"}) will even be better since I don't see a situation that email will be None if you make it as a required field.

General reference for this can be found at https://docs.pydantic.dev/latest/concepts/models/#required-fields.

Just be aware this change will entails a change in the schema, making the field required in the schema as well, so you may need to issue a new patch version

yarikoptic commented 5 months ago

no -- it should be only if

roleName:
  - dcite:ContactPerson
candleindark commented 5 months ago

@yarikoptic I have submitted a draft PR to provide a model level validator to ensure that a contributor acting as a contact person has an email provided. Let me know if the PR is heading to the direction of solving the problem. If it is, I will finalize the PR by adjusting the tests, etc.

yarikoptic commented 5 months ago

closed by

next time @candleindark please use Closes #NUMBER in PR description.