teamtomo / cryotypes

Super-simple data structures for cryo-electron tomography data
Other
3 stars 2 forks source link

Rotation matrices in dataframes #21

Open alisterburt opened 1 year ago

alisterburt commented 1 year ago

Came up in discussion of #20 with @brisvag

I like the pattern used there of providing a utility function for generating the projection matrices rather than attempting to store them on the dataframe itself where they don't fit naturally

What do you think about using a similar pattern of using a utility function for generating the Rotation representation from a standard rep in the dataframe?

We should still have the constructor take in a Rotation object and deal with unstacking, this would get rid of the need for people to understand quite so much about the Rotation API and more easily follow its docs which don't deal with 'how to shove this thing into a dataframe' at all

brisvag commented 1 year ago

We should still have the constructor take in a Rotation object and deal with unstacking

I'm not sure I follow; do you want to allow passing in both a Rotation and a specific euler angle convention, but then store it internally with euler angles? Or do you mean something else with standard rep?

What I meant was more that we should keep the Rotation internally (since it's the more convenient to pass around, as it's a single column, and it has the least amount of conventions to keep track of (except for matrices, which are however a pain to keep around) - but that we should have a function that takes that column of single rotations and spits out a matrix, for those who prefer to do calculations more manually rather than relying on the Rotation.__mul__ and similar.

alisterburt commented 1 year ago

Okay so two main points:

Together this makes the overhead a little higher than I would like for understanding/working with the data structure

I'm suggesting:

brisvag commented 1 year ago

I'm not a big fan of Yet Another Euler Angles Convention to deal with, but I see your points. I love working with Rotations, but originally I thought that getting them in or out of dataframes would be less tedious, and it's not... So yeah, I think I'm ok with the change!

alisterburt commented 1 year ago

sweet! will leave this open and update the spec when I can, cheers!