BHoM / BHoM_Adapter

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

Fix distinct causes assigned properties to be lost #355

Closed IsakNaslundBh closed 1 year ago

IsakNaslundBh commented 1 year ago

Issues addressed by this PR

Closes #126

Finally fixing this old issue of better handling merging of properties on "same" object being pushed in, most common case being highlighted in the issue (Node with and without constraints).

Done by instead of call to Distinct, a call to GroupBy is made, after CopyBHoMProperties and any CopyPropertiesModules is called, making sure all relevant properties are merged over.

Also, update to how the ID assignment from any duplicate objects is made, as simply can rely on the group, rather then trying to re-fetch from the list based on another call with the comparer. This should be a bit more efficient than previous implementation.

Test files

Have added new PushTest doing exactly what is highlighted in the issue. Was failing before, but working after changes.

Also, added a new test to check that all pushed objects have ids assigned. Was working before as well, but wanted to add this additional case as I made some changes to how it was handled.

Changelog

Additional comments

IsakNaslundBh commented 1 year ago

@BHoMBot check compliance

bhombot-ci[bot] commented 1 year ago
@IsakNaslundBh 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`
IsakNaslundBh commented 1 year ago

@BHoMBot check compliance

bhombot-ci[bot] commented 1 year ago
@IsakNaslundBh 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`
IsakNaslundBh commented 1 year ago

@BHoMBot check unit-tests

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

@BHoMBot check unit-tests

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

@BHoMBot check unit-tests

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

@BHoMBot check unit-tests

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

@BHoMBot check compliance @BHoMBot check required

bhombot-ci[bot] commented 1 year ago
@IsakNaslundBh 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` - check `code-compliance` - check `documentation-compliance` - check `project-compliance` - check `core` - check `null-handling` - check `serialisation` - check `versioning` - check `installer` There are 34 requests in the queue ahead of you.
bhombot-ci[bot] commented 1 year ago
The check `code-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 1 year ago
The check `documentation-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 1 year ago
The check `project-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
FraserGreenroyd commented 1 year ago

@BHoMBot check core @BHoMBot check versioning

bhombot-ci[bot] commented 1 year ago
@FraserGreenroyd to confirm, the following actions are now queued: - check `core` - check `versioning`
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 serialisation @BHoMBot check null-handling

IsakNaslundBh commented 1 year ago

@BHoMBot check serialisation @BHoMBot check null-handling

FraserGreenroyd commented 1 year ago

@BHoMBot check serialisation @BHoMBot check null-handling

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

@BHoMBot check ready-to-merge

FraserGreenroyd commented 1 year ago

@BHoMBot check ready-to-merge

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