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

Clunky and ambiguous naming of wrapped templated classes #7

Open
joeljonsson opened this issue Jun 12, 2019 · 4 comments
Open

Clunky and ambiguous naming of wrapped templated classes #7

joeljonsson opened this issue Jun 12, 2019 · 4 comments
Assignees

Comments

@joeljonsson
Copy link
Collaborator

A quick fix might be to rename them to remove any ambiguity, e.g.,

FloatParticles -> Float32Particles
ShortParticles -> Uint16Particles

But ideally we want an interface / base class Particles which can hold data of different types. The same goes for converter and pixeldata.

@joeljonsson joeljonsson self-assigned this Jun 12, 2019
@cheesema
Copy link
Member

What about f32 and u16?

@cheesema
Copy link
Member

On the above, did you ever try having a more generic base class, then using an interface with .astype(), and then auto datatype detection?

As currently this results in the code being prone to seg faults when you forget to manage your types.

Especially since its not super transparent in the conversion step.

@cheesema
Copy link
Member

cheesema commented Jan 8, 2022

This is largely being solved by #52 ?

@joeljonsson
Copy link
Collaborator Author

Not really.. It makes conversion easier, but doesn't touch the class names. In answer to your previous post:

On the above, did you ever try having a more generic base class, then using an interface with .astype(), and then auto datatype detection?

This is possible, and fairly simple to implement as a python class. However, we would need another layer of wrapping around the library calls so that the ParticleData object is passed in instead of the parent class. This could be pretty neat, but requires some refactoring.

Perhaps the cleanest solution would be to just use numpy arrays, and initialize ParticleData objects internally that borrow the underlying data (similar to what we do with PixelData).

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

No branches or pull requests

2 participants