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 mmap_mode option to allow writing to mmaped PLY files #79

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

nh2
Copy link

@nh2 nh2 commented Jun 16, 2024

Hi, @chpatrick wrote this and we find it super useful for being able to write PLY files in-place with mmap.

We already use this for a year in production and want to upstream it.

I just rebased it onto your latest master.


📚 Documentation preview 📚: https://python-plyfile--79.org.readthedocs.build/en/79/

@dranjan dranjan self-requested a review June 16, 2024 21:33
@dranjan dranjan self-assigned this Jun 16, 2024
Copy link
Owner

@dranjan dranjan left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

What should happen if the user requests pass-through writing but the file can't be memory-mapped? I'm actually not sure. My gut instinct is we should fail explicitly, but that could be a problem if the user is only interested in one element and it's some other element that can't be memory-mapped.

To take this to the finish line,

I can take care of all that, but feel free to take a crack at it yourself. My time frame is approximately one week from now, but that could change due to external factors.

plyfile.py Show resolved Hide resolved
plyfile.py Show resolved Hide resolved
@dranjan
Copy link
Owner

dranjan commented Aug 5, 2024

This will be superseded by #81. (The commit will still be merged, just not through this pull request. 😸)

@dranjan dranjan merged commit 618b02e into dranjan:master Aug 5, 2024
7 checks passed
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.

3 participants