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

Add "binary" expression in file open of registration_with_OpenGR example #5290

Closed
wants to merge 5 commits into from
Closed

Add "binary" expression in file open of registration_with_OpenGR example #5290

wants to merge 5 commits into from

Conversation

maximecharriere
Copy link
Contributor

Summary of Changes

registration_with_OpenGR example can't be compilled without this small change:

std::ifstream input(fname_in1);

to

std::ifstream input(fname_in1, std::ios::binary);

data/hippo1.ply and data/hippo2.ply are in binary_little_endian format. So read_ply_points can't read them if they aren't opened in binary mode.

Also add a specific error if a file can't be open and a new argument to pass the output filename

Release Management

  • Affected package(s): Point_set_processing_3

`data/hippo1.ply` and `data/hippo2.ply` are in binary_little_endian format. So `read_ply_points` can't read them if they aren't opened in binary mode.

Also add a specific error if a file can't be open and a new argument to pass the output filename
Add the
```c++
input.open(fname_in2, std::ios::binary);
```
expression in `registration_with_pointmatcher` and `registration_with_opengr_pointmatcher_pipeline` examples
Copy link
Contributor

@sgiraudot sgiraudot left a comment

Choose a reason for hiding this comment

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

I agree with the proposed change (adding the std::ios::binary tag) but the diff is way too large for such a small change. Please rewrite your commit without reindenting the whole files you modify (and please use spaces and not tabs for consistency with the rest of the files).

@maxGimeno maxGimeno modified the milestones: 5.1.2, 5.3-beta Dec 17, 2020
@MaelRL MaelRL added the Not yet approved The feature or pull-request has not yet been approved. label Dec 21, 2020
@maximecharriere
Copy link
Contributor Author

I have revert my changes and only add the binary parameter.

@MaelRL
Copy link
Member

MaelRL commented Dec 22, 2020

@maxGimeno If you change the milestone to 5.3, the PR loses its purpose since this was fixed in the IO PR already.

@lrineau
Copy link
Member

lrineau commented Dec 22, 2020

@maxGimeno If you change the milestone to 5.3, the PR loses its purpose since this was fixed in the IO PR already.

As the base branch of the PR includes commits from master only (like ef13599), we cannot merge this PR to anything but master, anyway.

@MaelRL
Copy link
Member

MaelRL commented Jan 5, 2021

#5323 has the same diff, but targets 5.1. This PR can probably be closed.

@sgiraudot
Copy link
Contributor

Sorry, I completely forgot about this PR, it could also have been rebased…

@MaelRL
Copy link
Member

MaelRL commented Feb 8, 2021

Replaced by #5323.

@MaelRL MaelRL closed this Feb 8, 2021
@MaelRL MaelRL added Replaced and removed Not yet approved The feature or pull-request has not yet been approved. labels Feb 8, 2021
@maximecharriere maximecharriere deleted the registration_with_OpenGR branch February 12, 2021 15:42
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