"Split arrays into separate tables" if there are less than 5 elements in array

[sorenabell]

No description provided.

[sorenabell]

@jpmckinney @sabahfromlondon this rule seems more logical and is not difficult to implement.
Let us know whether we should change this rule for splitting tables.

Also, whether 5 and fewer elements or just fewer than 5 elements should cause the split.

[jpmckinney]

this rule seems more logical and is not difficult to implement

To clarify, can you express the rule?

Also, whether 5 and fewer elements or just fewer than 5 elements should cause the split.

Which one is currently implemented?

[sabahfromlondon]

@jpmckinney for context what is happening in the flatten tool is that if there are fewer than 5 rows in a table then these are automatically rolled up into the main table - we specified that as a requirement.

However, on the interface the user is told that there is an array, but there is no option to split out the array. I think to a user, this is confusing and might even look like a bug.

We have two options:

1. We keep the fields rolled up, but need to provide the user with the option to split out the fields via the toggle switch.
2. We have the same behaviour for all arrays no matter now many fields in the array and automatically split out all tables.

My suggestion is to go with the first option.

[sorenabell]

@jpmckinney The current rule is "Split arrays into separate tables if there are 5 or more elements in the array".
But you mentioned that you want the opposite to be happening.

[jpmckinney]

@sorenabell And from your comment, which rule seems more logical: the current rule (never split if < 5, offer to split if >= 5) or the other rule (never always split if >=5, offer to split if < 5)?

[sabahfromlondon]

@jpmckinney

@jpmckinney The current rule is "Split arrays into separate tables if there are 5 or more elements in the array".
But you mentioned that you want the opposite to be happening.

I'm just trying to address the issue with the UI which is confusing. Telling the user that there is an array in the table and then no other information, is likely to lead to confusion and potentially the user thinking something is wrong with the tool.

I guess a third option is just to add an extra sentence for this instance to say: "The array has fewer than five fields, so has not been split out into a separate table."

By the way @sorenabell the sentence "There is one array in this tables" should be "table".

[jpmckinney]

Yes, we can clarify the language. I also just want to understand which behavior we actually want.

[sorenabell]

@sabahfromlondon This small text issue was fixed.

[jpmckinney]

From our call today, the new logic should be:

• Never merge a child table into a parent table if any parent object has >= [threshold] child objects
• Offer the option to merge a child table into a parent table if all parent objects have < [threshold] child objects

Regarding the choice of operators (>= and < versus > and <=), we can continue using the same operators we currently do. From my reading, we do should_split = len(item) >= self.table_threshold

This should address one of the causes of #303, where the problem was that merging the child table would cause the creation of a very wide table.

For now, the default option can be that all child tables are split. (In future, we might want to test whether users prefer to have single-value children merged into parent tables by default.)

We also agreed that, on each screen (i.e. for each parent table), the split behavior will be all-or-nothing: that is, either all child tables are merged, or all child tables are split. Per-child-table controls are expected to be too complicated for the user. (In future, we might want to test whether users prefer to have per-child-table controls, like when removing tables.)

[jpmckinney]

To clarify, regarding the all-or-nothing implementation, if one child table A is above the threshold, and the other child tables (B, C) are below the threshold, what happens?

1. The user has no choice. Since one table is above the threshold, all child tables must be split out.
2. The user can decide whether B and C are both merged, or both split out. A is always split out.

[sorenabell]

The second option seems to be correct:

• The user can decide whether B and C are both merged, or both split out. A is always split out.

[jpmckinney]

Great!

[sorenabell]

@jpmckinney just re-confirming and re-phrasing once more:

All child tables should be split (regardless of the number of elements in the array).

• If there are fewer than [threshold] elements, then we should offer the merge/unsplit option.
• If there are more than [threshold] elements, then we should not offer the merge at all and leave all the child tables split.

Is that correct?

[jpmckinney]

Yes. The second bullet should be "equal to or greater than", to cover the "at threshold" case.

[sorenabell]

Wireframes: https://whimsical.com/ocp-ft-split-tables-Sf2pYFG2vYNjS8hJPUdVJw

[sorenabell]

@jpmckinney Should CLI mimic the splitting behavior discussed in this issue?

[jpmckinney]

If the CLI can be more flexible, it would be better.

Otherwise, the CLI should behave the same as the web version.

[sorenabell]

I've created a separate issue for CLI open-contracting/spoonbill#176

[sorenabell]

The UI for the new splitting behavior was updated on the staging website. Please check.

[jpmckinney]

@sabahfromlondon Can you check this issue?

[sabahfromlondon]

@sorenabell I'm testing with this file (spoonbill-test.txt) and the table contracts_relatedProcesses should be able to be merged into the main table I think. The array only has one related process.

Also I'm not sure why the styling for the pop up box is different. The wireframes were only created to describe the requirement for the behaviour. We want the styling to be as per the rest of the tool.

Finally, the text in the pop up box does not match what was in the wireframe:

Here is a screenshot of how I wanted the text. I wanted to be specific about which tables can't be merged, rather than saying "some tables" as it is too vague.

[sorenabell]

This was updated. Currently on staging.

[sabahfromlondon]

@sorenabell I've been testing this with the following file created by @yolile
spoonbill-test (1).zip and I don't think that the behaviour is correct.

In the file attached the relatedProcesses array is below the threshold (=<5), but the pop up says that it can't be rolled up into the main table. The behaviour we wanted was to not allow arrays that were above the threshold only.

In any case, @jpmckinney and I discussed this and we do think that this issue is only likely to affect a very small number of users, that it's not something we want to focus lots of resource on.

As a result, I suggest that we leave the behaviour as is and if it becomes a problem for users then we can re-surface this ticket.

See screenshots: