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.

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

Updated the profiles to reflect categories and labeled missingness #70

Merged
merged 4 commits into from
Jun 12, 2024

Conversation

roll
Copy link
Member

@roll roll commented Jun 5, 2024

This is a follow-up of frictionlessdata/datapackage#68

@roll roll requested review from khusmann and peterdesmet June 5, 2024 08:56
@roll
Copy link
Member Author

roll commented Jun 5, 2024

@peterdesmet
@khusmann
Can you please take a look?

package.json Outdated Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Outdated Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Outdated Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Outdated Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Outdated Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Show resolved Hide resolved
profiles/source/dictionary/schema.yaml Outdated Show resolved Hide resolved
label:
type: string
value:
type: integer
Copy link
Member

Choose a reason for hiding this comment

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

@khusmann do I understand correctly that for an integer field, defining value in categories as something other than an integer, it would have to generate a validation error?

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is this is the point of the type-based version (moving it to the logical level). Am I correct?

Copy link

cloudflare-workers-and-pages bot commented Jun 5, 2024

Deploying datapackage with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2683e92
Status: ✅  Deploy successful!
Preview URL: https://425eb3eb.datapackage.pages.dev
Branch Preview URL: https://categories-update-profiles.datapackage.pages.dev

View logs

@roll
Copy link
Member Author

roll commented Jun 5, 2024

Thanks a lot @peterdesmet! I wanted to fix it too fast so made quite a lot of inaccuracies but now it's fixed

type: object
required:
- value
properties:
Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure. Defining the properties (value, label) doesn't restrict publishers to add more properties (e.g. description), correct?

Copy link
Member Author

@roll roll Jun 5, 2024

Choose a reason for hiding this comment

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

Yes, all the object are open for custom props unless it has additionalProperties: false (in-general that was the idea of an "open" to custom props specs)

@peterdesmet
Copy link
Member

@khusmann a question regarding alignment with the rest of Data Package: would it be beneficial/clearer if value and label were titled name and title or do we want dedicated terms?

@roll roll merged commit 5e15e11 into main Jun 12, 2024
2 checks passed
@roll roll deleted the categories-update-profiles branch June 12, 2024 09:03
@roll
Copy link
Member Author

roll commented Jun 12, 2024

I'm merging to fix the discrepancy between the spec ad the profile. Let's open an additional issue/discussion if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants