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

Breaking: Turn Z component into a generic property #20

Closed
wants to merge 3 commits into from

Conversation

josfeenstra
Copy link

Closes #18

Hi Hugo! During my work at Foresight we required a TIN for interpolating various attributes.

This change would allow storage & interpolation of any additional attributes.

Breaking changes

  • Points in general are now modeled as tuples rather than arrays, so the last component can be generic.
  • In situations with sizable generics, cloning starts to become more expensive.

Self diagnose

  • Names & api could use some work
  • Requires some more examples using different generics, implementing Attr.
  • Attr should be easier to implement. A few less traits are probably possible.

@hugoledoux
Copy link
Owner

hmmmm, quite a big change, I need some time and some further explanations. Especially that startin is also having python and C/Julia bindings, I don't want to break things that work at the moment.

I agree that having 1+ attributes would be good: lidar points having classification, intensity, etc.

But storing in tuples means loads of cloning in practice, as you mentioned... Can't we do better?

One option would be to store each point as a Vec (which contains [x, y, attr1, attr2, attr3, ...]), but that would mean all attributes are f64. Which is not super useful I guess?

One question: with your changes, how would I store one f64 (elevation) and one classification (integer). Would that be possible without modifying lib.rs?

But nice that you are interested in using startin!

@hugoledoux hugoledoux closed this Oct 7, 2024
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.

2 participants