Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Diffing_Engine: add "PropertiesToInclude" feature; control over max no of DifferentProperties; refactor #1900

Merged
merged 21 commits into from
Aug 3, 2020
Merged

Diffing_Engine: add "PropertiesToInclude" feature; control over max no of DifferentProperties; refactor #1900

merged 21 commits into from
Aug 3, 2020

Conversation

alelom
Copy link
Member

@alelom alelom commented Jul 13, 2020 •
edited
Loading

NOTE: Depends on

BHoM/BHoM#948
While not directly depending on it, BHoM/TriangleNet_Toolkit#40 is required for some of the test scripts to run smoothly.

Issues addressed by this PR

Closes #1711
Closes #1866

Test files

https://burohappold.sharepoint.com/:f:/s/BHoM/EpqHO1Ooxv5OqIwXsQvgFhsB1NwJEmkKhmNkZZ7hwLgjmQ?e=K7gQ23
There are various test files in this folder. They cover a good variety of cases for the Diffing.

I'm going to work tomorrow with @kayleighhoude and @Stepsan to test some of those.
@al-fisher perhaps we could test together what is left after that.

image

Changelog

  • Feature: added an "interface method" for diffing with a complex underlying logic to dispatch to the most appropriate Diffing method. This is intended to simplify the usage in scripts. Different Warnings/Notes guide the user in resolving possible problems depending on the inputs.
  • Feature: stop detecting property differences when 'MaxPropertyDifferences' limit is reached
  • Feature: simpler workflow where the Diffing can be done without Revisions. This obviously do not return any ModifiedObjects. See script.
  • Feature: Diffing can now be done detecting changes only on selected Properties ('PropertiesToConsider`)

@alelom alelom requested review from adecler and al-fisher as code owners July 13, 2020 14:08
This was referenced Jul 13, 2020
Merged
Closed
@alelom alelom self-assigned this Jul 13, 2020
@alelom alelom added type:feature New capability or enhancement size:M Measured in hours labels Jul 13, 2020
@alelom alelom requested a review from kayleighhoude July 14, 2020 20:22
@Stepsan Stepsan self-requested a review July 15, 2020 09:52
alelom added 16 commits July 15, 2020 12:50
@alelom
Added feature: DiffOneByOne. DifferentPorperites() now filters out pr…
d235ea2 
…operties differences based on DiffConfig.PropertiesToConsider.
@alelom
Merge branch 'master' into Diffing_Engine-#1866-PropertiesToInclude-r…
f2d3d0d 
…efactor
@alelom
Description
0a82243
@alelom
Added null check in ListModifiedProperties
4853822
@alelom
DifferentProperties-modifiedProperties: better names for props like F…
056949d 
…ragment params/CustomData
@alelom
Remove useless Create for the DiffConfig, and removed default params …
e6f0b42 
…for the one left
@alelom
DifferentProperties() - Feature: CustomData to ignore. Fix: for "pa…
7e3ff36 
…rameter" Fragments, display the name (hard code fix for Revit)

CustomData to ignore by defaults ignores RenderMesh.
@alelom
Switched to DeepClone for all DiffConfigs for immutability in UI.
5f96325
@alelom
Added Create() method for DiffConfig to remedy the bug in BHoM UI (se…
9e407bc 
…e description)

Bug in the auto Create() method ("auto-property initialisers for ByRef values like lists do not populate default values") is resolved.
@alelom
Sometimes I hate BHoM
50ad590
@alelom
Missing propagation of DiffConfig in SetHashFragment
2785d07
@alelom
Correction in DifferentProperties: now correctly remove RenderMesh
e55b92b
@alelom
Deepclone in DifferentProperties to ensure object immutability in UI
665cb3c
@alelom
Added filterName input in ListModifiedProperties
bb608e2
@alelom
Minor correction for key name on DiffWithFragmentId
976ae7b
@alelom
Merge branch 'master' into Diffing_Engine-#1866-PropertiesToInclude-r…
71326d8 
…efactor
@alelom
Copy link
Member Author

alelom commented Aug 3, 2020

@al-fisher this needs to be merged - we've been testing it for long now, as we had to use it for AECdeltas, also it's been pending for a bit too long I think

@alelom alelom mentioned this pull request Aug 3, 2020
Merged
al-fisher
al-fisher approved these changes Aug 3, 2020
View reviewed changes
Copy link
Member

@al-fisher al-fisher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - thanks @alelom
Has been tested in the field - Let's merge.

@al-fisher
Copy link
Member

/azp run BHoM_Engine.CheckInstaller

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alelom alelom mentioned this pull request Aug 3, 2020
Closed
@al-fisher al-fisher merged commit 5cefca2 into master Aug 3, 2020
@al-fisher al-fisher deleted the Diffing_Engine-#1866-PropertiesToInclude-refactor branch August 3, 2020 13:16
@alelom alelom mentioned this pull request Mar 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@al-fisher al-fisher al-fisher approved these changes

@adecler adecler Awaiting requested review from adecler adecler is a code owner

@kayleighhoude kayleighhoude Awaiting requested review from kayleighhoude

@Stepsan Stepsan Awaiting requested review from Stepsan

Assignees

@alelom alelom

Labels
size:M Measured in hours type:feature New capability or enhancement
Projects
None yet
Milestone
No milestone
2 participants
@alelom @al-fisher