Conversation
mberz
left a comment
mberz
left a comment
There was a problem hiding this comment.
I think we should review the concept again. Specifying all properties again does not use the power of the SH class at all from a users perspective.
Passing the SH class instead of creating it within the function would make things much easier in my opinion.
allow to pass sh data with dimensions < 3
overwrite basis_type setter and freq_raw
f-brinkmann
left a comment
f-brinkmann
left a comment
There was a problem hiding this comment.
Thanks for the effort. Functionality is great to have and looks good to me. I only marked two points for reducing code for now.
spharpy/sht.py
Outdated
| # move spherical samples to -2 | ||
| data = np.moveaxis(data, axis, -2) | ||
|
|
||
| # perform transform | ||
| data_nm = np.tensordot(Y_inv, data, [1, -2]) | ||
|
|
||
| # move SH channels to -2 | ||
| data_nm = np.moveaxis(data_nm, 0, -2) |
There was a problem hiding this comment.
We have matrix multiplication implemented in pyfar arithmetics. If we use this, we can maybe avoid moving the axis by passing the axes parameter.
spharpy/sht.py
Outdated
| if isinstance(signal, Signal): | ||
| sh_signal = SphericalHarmonicSignal( | ||
| data=data_nm, | ||
| basis_type=spherical_harmonics.basis_type, | ||
| normalization=spherical_harmonics.normalization, | ||
| channel_convention=spherical_harmonics.channel_convention, | ||
| condon_shortley=spherical_harmonics.condon_shortley, | ||
| sampling_rate=signal.sampling_rate, | ||
| fft_norm=signal.fft_norm, | ||
| is_complex=signal.complex, | ||
| comment=signal.comment) | ||
| elif isinstance(signal, TimeData): | ||
| sh_signal = SphericalHarmonicTimeData( | ||
| data=data_nm, | ||
| times=signal.times, | ||
| basis_type=spherical_harmonics.basis_type, | ||
| normalization=spherical_harmonics.normalization, | ||
| channel_convention=spherical_harmonics.channel_convention, | ||
| condon_shortley=spherical_harmonics.condon_shortley, | ||
| comment=signal.comment, | ||
| is_complex=False) | ||
| elif isinstance(signal, FrequencyData): | ||
| sh_signal = SphericalHarmonicFrequencyData( | ||
| data=data_nm, | ||
| frequencies=signal.frequencies, | ||
| basis_type=spherical_harmonics.basis_type, | ||
| normalization=spherical_harmonics.normalization, | ||
| channel_convention=spherical_harmonics.channel_convention, | ||
| condon_shortley=spherical_harmonics.condon_shortley, | ||
| comment=signal.comment) |
There was a problem hiding this comment.
After we have implemented the from_defition methods for the SHAudio classes, this could probably be shortened significantly.
4 participants
Which issue(s) are closed by this pull request?
Closes #133
Changes proposed in this pull request:
transforms?