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

Fix example mesh_3D_gray_vtk_image #7824

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

ange-clement
Copy link
Member

@ange-clement ange-clement commented Oct 24, 2023

Summary of Changes

Fixes example mesh_3D_gray_vtk_image.
Changed : Meshing all value under isovalue --> Meshing all values over isovalue.

Release Management

  • Affected package(s): Mesh_3
  • Issue(s) solved (if any):
  • Feature/Small Feature (if any): n/a
  • License and copyright ownership: no change

Copy link
Member

@janetournois janetournois left a comment

Choose a reason for hiding this comment

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

Using Less or More actually depends on the input data.
Using More is the default behavior when using isovalue only, and using Less gives another example. This should be better documented

@sloriot sloriot added Batch_1 First Batch of PRs under testing Under Testing and removed Batch_1 First Batch of PRs under testing labels Oct 31, 2023
@sloriot sloriot added Batch_1 First Batch of PRs under testing Under Testing and removed Batch_1 First Batch of PRs under testing labels Nov 28, 2023
@sloriot
Copy link
Member

sloriot commented Dec 7, 2023

Successfully tested in CGAL-6.0-Ic-122

@lrineau
Copy link
Member

lrineau commented Dec 7, 2023

@janetournois @ange-clement As far as I understand, that pull-request is debated. I will not merge it, even if it was tested successfully.

@sloriot
Copy link
Member

sloriot commented Dec 14, 2023

@janetournois what is the status of this PR?

@janetournois
Copy link
Member

This modifies the example of Mesh_3 to make it work with different data sets, and it does not bring any new info or feature, so imho it can be closed.

@sloriot sloriot closed this Dec 18, 2023
@sloriot sloriot reopened this Dec 18, 2023
@sloriot
Copy link
Member

sloriot commented Dec 18, 2023

What about adding an option in the parameter to pick less or more?

@lrineau lrineau modified the milestones: 5.5.4, 5.5.5 Feb 28, 2024
@lrineau lrineau removed this from the 5.5.5 milestone Jun 7, 2024
@lrineau lrineau changed the base branch from 5.5.x-branch to master June 10, 2024 07:33
@sloriot sloriot force-pushed the Mesh_3-fix_examples-GF branch from 30689f0 to 335f9b3 Compare November 5, 2024 16:30
@janetournois
Copy link
Member

Good idea @sloriot !
However meshing this data will fail with less=false

@sloriot
Copy link
Member

sloriot commented Nov 5, 2024

Good idea @sloriot ! However meshing this data will fail with less=false

which is why the default is true

@sloriot
Copy link
Member

sloriot commented Nov 8, 2024

Successfully tested in CGAL 6.1-Ic-14

@sloriot sloriot merged commit f7ac241 into CGAL:master Nov 8, 2024
8 of 9 checks passed
@sloriot sloriot deleted the Mesh_3-fix_examples-GF branch November 8, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants