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: Correctly pass tolerance for LineIntersection methods #2903

Merged

Conversation

IsakNaslundBh
Copy link
Contributor

@IsakNaslundBh IsakNaslundBh commented Sep 2, 2022

Issues addressed by this PR

Closes #2902

Fixes tolerances being correctly passed to method doing actual work in line intersections.

Test files

https://burohappold.sharepoint.com/:f:/s/BHoM/EuTzNW600kpFglxk53P9rlEBW6FsOhhmEEgKMHqCnCCLeQ?e=hhoZ9M

Changelog

Additional comments

Changed the name of the file from LineIntersections to LineIntersection (removing the plural s) as the code compliance accepts a trailing s on the method relative to the file name, but does not accept it the other way around.

@IsakNaslundBh IsakNaslundBh added the type:bug Error or unexpected behaviour label Sep 2, 2022
@IsakNaslundBh IsakNaslundBh self-assigned this Sep 2, 2022
@IsakNaslundBh IsakNaslundBh changed the title Correctly pass tolerance for LineIntersection methods Geometry_Engine: Correctly pass tolerance for LineIntersection methods Sep 2, 2022
@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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

@BHoMBot check code-compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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

  • check code-compliance

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check compliance

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

@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

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

@IsakNaslundBh 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

There are 5 requests in the queue ahead of you.

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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

bhombot-ci bot commented Sep 2, 2022

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.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check unit-tests

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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

  • check unit-tests

There are 18 requests in the queue ahead of you.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check required

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

@IsakNaslundBh 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

There are 1 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 about the changes (clear improvement), but as discussed with @IsakNaslundBh offline, the whole concept of line infinity looks pretty overcooked now, possibly we could get rid of useInfiniteLine argument and simply drive the queries by Line.Infinite? Of course not for this PR, more of a general thought.

@IsakNaslundBh
Copy link
Contributor Author

@BHoMBot check copyright-compliance
@BHoMBot check dataset-compliance
@BHoMBot check unit-tests
@BHoMBot check ready-to-merge

@bhombot-ci
Copy link

bhombot-ci bot commented Sep 2, 2022

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

  • check copyright-compliance
  • check dataset-compliance
  • check unit-tests
  • check ready-to-merge

There are 41 requests in the queue ahead of you.

@FraserGreenroyd
Copy link
Contributor

@IsakNaslundBh could we consider adding some Unit Tests for this in the future (happy for it to be a new issue if desired)?

@IsakNaslundBh
Copy link
Contributor Author

@IsakNaslundBh could we consider adding some Unit Tests for this in the future (happy for it to be a new issue if desired)?

Yes, we can. Will handle spearately. Issue raised here #2904

@IsakNaslundBh IsakNaslundBh merged commit 0fdb828 into main Sep 5, 2022
@IsakNaslundBh IsakNaslundBh deleted the Geometry_Engine-#2902-FixLineIntersectionTolerancePassing branch September 5, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error or unexpected behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Geometry_Engine: LineIntersection not passing tolerances properly
3 participants