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: slow read_off method #353

Merged
merged 7 commits into from
Nov 23, 2023
Merged

FIX: slow read_off method #353

merged 7 commits into from
Nov 23, 2023

Conversation

safesintesi
Copy link
Contributor

@safesintesi safesintesi commented Sep 9, 2023

Had to use this method for a project and it was incredibly slow.

Fixed changing the engine and tweaking the parameters so that no new import is required.

Also fixes:

  • comments in file
  • ignores edges that were previously read as faces
  • direct type casting instead of casting after creating the DataFrame
  • coercing error on obj.py
  • changed jit decorator to njit
  • little spelling mistake :)

I tried to modify the least I could.

Problems not solved:

  • OFF files actually can have heterogeneous shapes... :(

ADDED comment parameter to skip comment rows and end of line comments
CHANGED to c engine to increase drastically the performance
ADDED n_rows and REMOVED skipfooter to use c engine
ADDED dtype parameter to directly cast the element types
SET points read_csv to off buffer: no skip of first lines needed
pyntcloud/io/off.py Fixed Show fixed Hide fixed
pyntcloud/io/off.py Fixed Show fixed Hide fixed
Fixed typo in parameters name
Added n_points and n_faces default as 0
@safesintesi
Copy link
Contributor Author

Not sure how to act if a file exists and respects the standard but has no points and no shapes.
At first I added a check that raises a ValueError, but then I commented it out because returning an empty point cloud might be better.
Thinking about it, maybe a loop that loads a bunch of files shouldn't break just because one of them is empty.
Thoughts about this?

@daavoo
Copy link
Owner

daavoo commented Sep 18, 2023

Not sure how to act if a file exists and respects the standard but has no points and no shapes. At first I added a check that raises a ValueError, but then I commented it out because returning an empty point cloud might be better. Thinking about it, maybe a loop that loads a bunch of files shouldn't break just because one of them is empty. Thoughts about this?

I think it is ok (and would prefer it) to raise an exception. People could handle exceptions in their loop

pyntcloud/io/off.py Fixed Show resolved Hide resolved
@safesintesi
Copy link
Contributor Author

@daavoo done 🦤

@daavoo daavoo self-requested a review September 22, 2023 14:22
@safesintesi
Copy link
Contributor Author

Kindly pinging to remind someone about this <3

@tflidd
Copy link

tflidd commented Nov 23, 2023

Just on the commit f9bbff7, this works for me 👍

daavoo
daavoo previously approved these changes Nov 23, 2023
pyntcloud/io/obj.py Outdated Show resolved Hide resolved
Copy link
Owner

@daavoo daavoo left a comment

Choose a reason for hiding this comment

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

Thanks @safesintesi

@daavoo daavoo merged commit cab0c7f into daavoo:master Nov 23, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants