fix: 🐛 change fields_match to be an array in extract_resource_properties()

[signekb]

Description

It's supposed to be a string, but there's an error in the JSON schema. According to a PR in the data package repo, this should be fixed in datapackage V2.1 🤞

I encountered this issue bc the website build fails bc there was an error in resources.qmd when writing the properties.

This PR needs a quick review.

Checklist

• Ran just run-all

[martonvago]

Ah we're seeing this now because the camel/snake case fix was merged in!

But do we want to change the type of fields_match in TableSchemaProperties to array as well? Because now it's type hinted as a string (well, FieldsMatchType). There are also some other references to fields_match in the tests.

If we don't want to make a bigger change to the "wrong" type in TableSchemaProperties, we could

• not set fields_match in Sprout until the issue is fixed
• fix it ourselves in the schema, but not by editing the JSON directly, but by resetting that one property in check_properties / check_resource_properties when the schema is loaded

[signekb]

Ahhh, I was wondering why it didn't show up earlier. That makes sense.

I suspect that it might take a while before the PR that fixes this isse is merged, unfortunately. At least, it's been open since Oct 2024. But you're absolutely right, we should align however we decide to fix this across Sprout.

Did we decide on anything more general about how we handle these situations after we found that the list field type was missing in the schema (#1064)? There, we removed the field type instead of editing the schema. Would an equivalent in this situation be to just not set it in Sprout for now then (potentially with a TODO comment about this)? Don't think we use it a lot anyways.

What do you think, @martonvago?

[martonvago]

@signekb Yeah, I think we don't use it in Sprout for anything, so it's okay to not set it at all for now. (We should set it eventually only because Sprout doesn't use the default value for it, which is not obvious if datapackage.json is ever used/shared outside of Sprout.)

[signekb]

@martonvago Hmmm, maybe we should set it in Sprout as the default then? With the default being the array bc that's what it is according to the schema?

[martonvago]

Hmm, I don't feel strongly about either deleting it or changing it to an array.
I guess your argument about being consistent with how we dealt with the list type missing is convincing though. So I'm happy to change it to an array!

[lwjohnst86]

I can't tell if there is a resolution to the discussion so @signekb I'll let you merge in when you are ready/the discussion has been resolved :)

[signekb]

@lwjohnst86 We briefly discussed it at the meeting this morning, and I think we agreed on changing it to an array to fit the current schema even though it's incorrect and should be fixed in datapackage V2.1. What do you think?

[lwjohnst86]

I think we'll just have to fix it until they formally correct the issue. They have a call for frictionless on June 27 where I think they'll talk about the next version status

[signekb]

Ah, ok, should we attend that meeting?

[lwjohnst86]

@signekb I might, haven't decided though.

[lwjohnst86]

I'm going to merge this in, since the docs workflow keeps failing.