search

imglib / imglib2-realtransform

Spatial transformations for ImgLib2
Other
7 stars 7 forks source link

Position fields, mixed transform, and transition #11

Closed axtimwalde closed 6 years ago

axtimwalde commented 6 years ago

This introduces real and discrete accesses over single axis positions, a real transform that mixes two real transforms and an n-dimensional transition transform that transitions between two n-1 dimensional transforms (which uses the mixing transform). @tpietzsch @dietzc @StephanPreibisch objections?

tpietzsch commented 6 years ago

@axtimwalde Seems very useful! I would rather put PositionRandomAccessible, RealPositionRealRandomAccessible into imglib2 core.

Are you sure you want to not support the apply( float[] ... ) version in InterpolatedRealTransform?

axtimwalde commented 6 years ago

I would rather put PositionRandomAccessible, RealPositionRealRandomAccessible into imglib2 core

I thought so too but then couldn't decide into which package to to put it net.imglib2.position ?

axtimwalde commented 6 years ago

Are you sure you want to not support the apply( float[] ... ) version in InterpolatedRealTransform?

No, that is lazy error. Thanks for spotting this. I am actually angry that we have the float[] methods in the first place. Never use them. What do you think about deprecating them? Like in many other cases, I have the choice now to carry around some superfluous float[] arrays or implement it inefficiently by copy+conversion. Doing the latter now.

tpietzsch commented 6 years ago

I thought so too but then couldn't decide into which package to to put it net.imglib2.position ?

sounds good!

tpietzsch commented 6 years ago

No, that is lazy error. Thanks for spotting this. I am actually angry that we have the float[] methods in the first place. Never use them. What do you think about deprecating them? Like in many other cases, I have the choice now to carry around some superfluous float[] arrays or implement it inefficiently by copy+conversion. Doing the latter now.

I'm fine with deprecating them.

How about you put the inefficiently copy+conversion float[] method into the interface as a (deprecated) default method. Then we could get rid of all implementations right away...

axtimwalde commented 6 years ago

How about you put the inefficiently copy+conversion float[] method into the interface as a (deprecated) default method. Then we could get rid of all implementations right away...

That would make most of the existing implementations even worse. Let me think about it a little longer...

axtimwalde commented 6 years ago

@tpietzsch I did as you said https://github.com/imglib/imglib2-realtransform/pull/12 , but did not yet delete all existing implementations because we can do that easier when removing the method.

tpietzsch commented 6 years ago

what about putting PositionRandomAccessible, RealPositionRealRandomAccessible into imglib2 core? Package net.imglib2.position sounds fine to me

axtimwalde commented 6 years ago

@tpietzsch already done, following your earlier positive comment https://github.com/imglib/imglib2/commit/59cf72243782679af46aa29928879504ddb36f29

tpietzsch commented 6 years ago

oh sorry, I missed that!