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.

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

Geometry_Engine: Add methods to assist with tagging workflows #3102

Merged
merged 6 commits into from
Aug 28, 2023

Conversation

vietle-bh
Copy link
Contributor

@vietle-bh vietle-bh commented Jul 6, 2023

NOTE: This is #3035 reopened.

Issues addressed by this PR

Closes #3033, Closes #3034

Added new methods such as:

  • Get the smallest possible angle between 2 vectors
  • Thickening a line into a polyline
  • Find line intersections between a polyline and a line
  • Find the distance between 2 points along a direction vector
  • Converting between degree and radian values
  • Copy property values between 2 objects where the property names match
  • Shifting and splitting a list

Test files

Test_script.zip

Changelog

Added engine methods for new use cases.

Additional comments

@vietle-bh vietle-bh self-assigned this Jul 6, 2023
@vietle-bh vietle-bh added the type:feature New capability or enhancement label Jul 6, 2023
@vietle-bh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check core
@BHoMBot check null-handling
@BHoMBot check serialisation

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 7, 2023

@vietle-bh 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 core
  • check null-handling
  • check serialisation

There are 8 requests in the queue ahead of you.

@vietle-bh
Copy link
Contributor Author

@BHoMBot check unit-tests
@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 7, 2023

@vietle-bh to confirm, the following actions are now queued:

  • check unit-tests
  • check versioning

There are 40 requests in the queue ahead of you.

@vietle-bh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Jul 7, 2023

@vietle-bh to confirm, the following actions are now queued:

  • check unit-tests

@vietle-bh vietle-bh force-pushed the Revit_Tools-#605-AddTaggingTool branch from 64af09c to 0b17eb3 Compare August 18, 2023 08:36
Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

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

I like the code, only a few minor tweaks suggested. However, please be so kind and provide a Grasshopper test script with the added methods - you are touching core projects of the entire BHoM here so extensive testing is a must. The script should ensure correctness of the methods as it will be serialised to serve as a unit test in the future.

BHoM_Engine/Modify/SplitAndRemoveAtIndexes.cs Outdated Show resolved Hide resolved
Geometry_Engine/Convert/ToXY.cs Outdated Show resolved Hide resolved
Geometry_Engine/Convert/ToXY.cs Outdated Show resolved Hide resolved
@vietle-bh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check core
@BHoMBot check null-handling
@BHoMBot check serialisation

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 23, 2023

@vietle-bh 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 core
  • check null-handling
  • check serialisation

@vietle-bh
Copy link
Contributor Author

@BHoMBot check compliance
@BHoMBot check core
@BHoMBot check null-handling
@BHoMBot check serialisation

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 24, 2023

@vietle-bh 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 core
  • check null-handling
  • check serialisation

@vietle-bh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 24, 2023

@vietle-bh to confirm, the following actions are now queued:

  • check unit-tests

There are 5 requests in the queue ahead of you.

@vietle-bh
Copy link
Contributor Author

@pawelbaran , I've added the GH test script to this PR's description.

@vietle-bh vietle-bh marked this pull request as ready for review August 24, 2023 10:53
@vietle-bh vietle-bh requested a review from pawelbaran August 24, 2023 10:53
@pawelbaran
Copy link
Member

@vietle-bh I have had a look at the test script you prepared - it is a good start, but definitely not sufficient (not enough cases, no verification). I quickly patched up an alternative how I would do it and uploaded it here. Let's have a chat tomorrow to coordinate on how to bring this PR over the line 💪

@pawelbaran pawelbaran requested a review from rwemay as a code owner August 25, 2023 11:19
@pawelbaran
Copy link
Member

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 25, 2023

@pawelbaran to confirm, the following actions are now queued:

  • check unit-tests

There are 28 requests in the queue ahead of you.

Copy link
Member

@pawelbaran pawelbaran left a comment

Choose a reason for hiding this comment

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

Happy to approve the PR following the code review, testing and unit test creation done together with @vietle-bh 👍 Good stuff!

@vietle-bh
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 25, 2023

@vietle-bh to confirm, the following actions are now queued:

  • check code-compliance
  • check documentation-compliance
  • check project-compliance
  • check core
  • check null-handling
  • check serialisation
  • check versioning
  • check installer

@vietle-bh
Copy link
Contributor Author

@BHoMBot check versioning

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 25, 2023

@vietle-bh to confirm, the following actions are now queued:

  • check versioning

There are 19 requests in the queue ahead of you.

@vietle-bh
Copy link
Contributor Author

Sorry @FraserGreenroyd , similar story here where the failed test isn't relevant to this PR. It's the unit-tests this time. Could we bypass this please?

@FraserGreenroyd FraserGreenroyd force-pushed the Revit_Tools-#605-AddTaggingTool branch from 32c6fa3 to 6ab5caf Compare August 28, 2023 10:32
@FraserGreenroyd
Copy link
Contributor

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 28, 2023

@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
Copy link
Contributor

@BHoMBot check core
@BHoMBot check null-handling
@BHoMBot check serialisation

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 28, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check core
  • check null-handling
  • check serialisation

@FraserGreenroyd
Copy link
Contributor

@BHoMBot check versioning
@BHoMBot check installer

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 28, 2023

@FraserGreenroyd to confirm, the following actions are now queued:

  • check versioning
  • check installer

@FraserGreenroyd
Copy link
Contributor

@BHoMBot this is a DevOps instruction. I am requesting neutral checks on: ready-to-merge, unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Aug 28, 2023

@FraserGreenroyd I have provided neutral checks to the checks requested. These checks will need to be run properly to obtain full results.

@FraserGreenroyd FraserGreenroyd changed the title Add new methods to Geometry_Engine & Bhom_Engine Geometry_Engine: Add methods to assist with tagging workflows Aug 28, 2023
@FraserGreenroyd FraserGreenroyd merged commit 03af47c into develop Aug 28, 2023
@FraserGreenroyd FraserGreenroyd deleted the Revit_Tools-#605-AddTaggingTool branch August 28, 2023 11:25
@bhombot-ci bhombot-ci bot mentioned this pull request Sep 11, 2023
@bhombot-ci bhombot-ci bot mentioned this pull request Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
Development

Successfully merging this pull request may close these issues.

Additional methods for Bhom_Engine Additional methods for Geometry_Engine
3 participants