BHoM / BHoM_UI

GNU Lesser General Public License v3.0
9 stars 5 forks source link

Fix explode of Custom Object with a nested list property #469

Closed FraserGreenroyd closed 1 year ago

FraserGreenroyd commented 1 year ago

Fixes https://github.com/BHoM/BHoM_Engine/issues/2674

Test Files

Available here. The JSON used has come from @tg359 on the linked issue which this PR aims to resolve.

On alpha:

image

On this PR:

image

If testing using the provided test script please make sure you redrag the output of FromJson into the Explode component to get it to update. Alternatively, drop a new Explode component on the canvas and compare the cached one with the one from this PR.

E.G:

image

FraserGreenroyd commented 1 year ago

@BHoMBot check compliance

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance`
FraserGreenroyd commented 1 year ago

@BHoMBot check compliance

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `branch-compliance` - check `dataset-compliance` - check `copyright-compliance`
bhombot-ci[bot] commented 1 year ago
FAO: @FraserGreenroyd @FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate. The check they wish to have dispensation on is project-compliance. If you are providing dispensation on this occasion, please reply with: > @BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. `16280314097`
FraserGreenroyd commented 1 year ago

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 16280314097

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd I have now provided a passing check on reference `16280314097` as requested.
FraserGreenroyd commented 1 year ago

@BHoMBot check versioning

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `versioning`
FraserGreenroyd commented 1 year ago

@BHoMBot check core

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `core`
FraserGreenroyd commented 1 year ago

@BHoMBot check versioning

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `versioning`
FraserGreenroyd commented 1 year ago

@BHoMBot check installer @BHoMBot check copyright-compliance @BHoMBot check project-compliance

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `installer` - check `copyright-compliance` - check `project-compliance`
bhombot-ci[bot] commented 1 year ago
FAO: @FraserGreenroyd @FraserGreenroyd is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate. The check they wish to have dispensation on is project-compliance. If you are providing dispensation on this occasion, please reply with: > @BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. `16281528423`
FraserGreenroyd commented 1 year ago

@BHoMBot this is a DevOps instruction. I am authorising dispensation to be granted on check ref. 16281528423

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd I have now provided a passing check on reference `16281528423` as requested.
adecler commented 1 year ago

To summarise the discussion I just had with @FraserGreenroyd , I feel that this PR doesn't address the underlying problem of the deserialisation of lists of lists that comes from an external serialisation source. So this PR runs the risk of masking the problem instead of fixing it.

See below the resulting serialisation of a List of lists fully created within the BHoM vs the one coming coming from a deserialisation of external json:

image

As you can see, the later generates 'messier' json independently of the Explode issue addressed here. If that deserialisation is resolved, the Explode issue should go away.

We might still need to improve the Explode component if we find other cases of List<T> represented as List<object> or even lists of objects that contain multiple types (including List<T>) but then the PR should be done addressing the problem in its larger context, not just this one caused by serialisation.

FraserGreenroyd commented 1 year ago

Closing this in favour of this PR - we can reopen this if needed