FIX: get unique, with conflicting meta-data

[adelavega]

Fixes #747

This is a cheap fix for a bug that occurs when a .tsv meta-data entry (i.e. column description) matches a common BIDS entity.

For example, if you call layout.get_tasks(), it will first get all unique values for the task entity. However, if index_metadata=True, that includes all instances of task in .json sidecars for TSVs as well. However, .json sidecars for .tsv files are a different type of meta-data that describes the columns in TSV files, not the entities of the corresponding file.

Thus, get_tasks will fail because it tries to take a set on a list that includes a dict value. Here, I simply excluded dict values from that operation.

[adelavega]

Looking at this a bit more, the issue is that all meta-data is stored as an Entity, with the attribute is_metadata=True.

This is fine, except that .json sidecars for .tsv files are a different kind of meta-data. It's meta-data associated with columns, not files.

One option is to not index meta-data for .tsv files at all. However, then f.get_metadata() would not work for these files.

Alternatively, we can add another column to Entity which describes if this is column or file meta-data (is_column?). This would be excluded from get_entities but not from get_metadata.

[adelavega]

@effigies does the inheritance principle apply to tsv file meta-data? If not then that's an argument that pybids should just ignore this type of meta-data from indexing.

[effigies]

As far as I know, it should apply.

[adelavega]

I suppose I could imagine how the inherentice principle might apply here, if all event files have the same columns across all subjects.

It's still weird to me that pybids would index this meta-data in a way that lets you filter on it.

For example, it makes no sense to filter tsv files based on the Description of a specific column within it.

[oesteban]

This is related to #684. A quick&dirty, ad-hoc workaround would be to filter the retrieved metadata to only provide str, along the same lines of #682 for runs.

EDIT: But yes, I agree with @effigies that there must be another source of metadata for the entity "session" that is inserting a dict in the db.

[adelavega]

@oesteban that's basically what I did but only excluded dicts.

The dicts are coming from tsv sidecars, since they allow dicts. If you describe a column called "session" in participants.json then there will be a match for the "session" entity that is a dict in the db.

[oesteban]

If you describe a column called "task" in participants.tsv then there

will be a match for the task entity that is a dict. Yes, literally any metadata that defines a field whose name is a bids entity. This would be very problematic if instead of a dict, such "task" field were a valid str task identifier (e.g. "faketask"). Now, instead of an error, faketask would be returned as one more task by get_tasks()

…

On Wed, Jun 30, 2021, 21:20 Alejandro de la Vega ***@***.***> wrote: @oesteban <https://github.com/oesteban> that's basically what I did but only excluded dicts. The dicts are coming from tsv sidecars, since they allow dicts. If you describe a column called "task" in participants.tsv then there will be a match for the task entity that is a dict. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#748 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAESDRREFANXCAXMOJR2DC3TVNVBFANCNFSM47RERXVA> .

[adelavega]

Yep.

Well if nobody has any objections to this fix, I'll merge it, but I think in the long run the solution is to amend meta-data to differentiate between those coming from tsv sidecars.

[effigies]

Yeah, I'm okay with this short term, but can we open an issue to make sure it doesn't get forgotten?

[oesteban]

Before that, it would also be interesting to make a decision regarding #694.

[adelavega]

I think #694 sufficient covers what I pointed out. I'm going to merge this as is, as its a relatively innocuous and in practice fixes the most glaring issues with this.