Reader for .ita files and test functions for it by h-chmeruk · Pull Request #839 · pyfar/pyfar

h-chmeruk · 2025-09-04T11:38:48Z

Which issue(s) will be closed by this pull request?

Replaces #720
Closes #206

Here is a draft implementation of test functions for the .ita Reader. If you have any suggestions or comments regarding the test functions or the read_ita function itself, please do not hesitate to write them.

h-chmeruk · 2025-09-04T14:01:37Z

pyfar/io/io.py

+        if ita_coordinates.cart.size == 0:
+            return pf.Coordinates()
+        if ita_coordinates.cart.shape[-1] != 3:
+            raise ValueError(


I also thought about implementing a function to test this ValueError, but I was unable to create an .ita file with an invalid number of dimensions (as there are enough exceptions for this case in MATLAB).
If anyone knows how to test this, I would appreciate some help :)

seems to be difficult, in this case you can skip the test, but we should keep the error handling here

sikersten

Thanks finalizing this!

Please check the reason for the failing tests, I think this may have to do with obsolete test filtes.
I would suggest removing all stars * before .ita in the docstrings, since these are strangly rendered: https://pyfar--839.org.readthedocs.build/en/839/modules/pyfar.io.html#pyfar.io.read_ita

sikersten · 2025-10-14T08:06:41Z

pyfar/io/io.py

+
+
+def read_ita(filename):
+    """Read a *.ita file from the ITA-toolbox.


It may be nice to link to the toolbox here (adding it as reference below): https://www.ita-toolbox.org/

sikersten · 2025-10-14T08:09:11Z

pyfar/io/io.py

+    Returns
+    -------
+    data : pyfar.Signal, pyfar.TimeData, pyfar.FrequencyData
+        The data contained in the *.ita file.


Suggested change

The data contained in the *.ita file.

The audio data contained in the *.ita file.

sikersten · 2025-10-14T08:10:05Z

pyfar/io/io.py

+
+
+def read_ita(filename):
+    """Read a *.ita file from the ITA-toolbox.


Suggested change

"""Read a *.ita file from the ITA-toolbox.

"""Read an *.ita file created using the ITA-toolbox.

sikersten · 2025-10-14T08:11:26Z

pyfar/io/io.py

+    return data, object_coordinates, channel_coordinates, metadata
+
+
+def _to_coordinates(ita_coordinates):


Suggested change

def _to_coordinates(ita_coordinates):

def _ita_to_pyfar_coordinates(ita_coordinates):

sikersten · 2025-10-14T08:14:53Z

pyfar/io/io.py

+
+    # checks if *.ita object is itaAudio or itaResult
+    if (hasattr(matlab_data, 'signalType')):  # itaAudio
+        fft_norm = 'none' if matlab_data.signalType == 'energy' else 'rms'


Maybe add a comment here, that this results from the ITA-toolbox logic?

sikersten · 2025-10-14T08:20:13Z

pyfar/io/io.py

+            weights=weights)
+
+    # import as spherical coordinates: r, theta, phi
+    if hasattr(ita_coordinates, 'sph'):


Is it possible that ita_coordinates has both attributes "cart" and "sph"? If not, this would need to be an elif connected to line 1035

I am not an expert on the ITA-Toolbox, but I think that ita_coordinates can have both attributes at the same time.

@mberz can you clarify? The files I saw, that contained both had identical data stored in different coordinate conventions, I think.

I can either be 'cart' or 'sph', similar to what we has before in our coordinates class.

sikersten · 2025-10-14T08:21:10Z

pyfar/io/io.py

+            ita_coordinates.sph[..., 0],
+            weights=weights)
+    else:
+        return pf.Coordinates()


I would suggest returning None here, since this object won't be of any use

sikersten · 2025-10-14T08:25:03Z

tests/test_io.py

+    """Tests correct exception raise for example *.ita files."""
+    filepath = os.path.join('tests', 'test_io_data', filename)
+    if (filename == 'freq_itaResult.ita' and data_type == 'signal'):
+        message = "The itaResult object can't contain a signal."


This message is not defined in the function?

sikersten · 2025-10-14T08:25:52Z

tests/test_io_read_ita_ahe.py

I think this file should not be here

sikersten · 2025-10-14T08:26:26Z

tests/test_io.py

Please move the added content to test_io_read_ita.py

Thank you for your review!
I just noticed that this file tests/test_io.py contains an outdated test function test_read_ita(filename, data_type) that we don't need. Therefore, I would simply remove this function from the file.

h-chmeruk · 2025-10-16T08:38:17Z

Thanks finalizing this!

* Please check the reason for the failing tests, I think this may have to do with obsolete test filtes.

* I would suggest removing all stars * before .ita in the docstrings, since these are strangly rendered: https://pyfar--839.org.readthedocs.build/en/839/modules/pyfar.io.html#pyfar.io.read_ita

Thanks!
Regarding the stars * before .ita in the docstring:
I could, of course, simply remove all stars, but I could also enclose the * .ita in backticks so that the text is rendered as * .ita.
Is this a better solution, or should I rather remove all asterisks before .ita?

(I've already tested it, it works, and it looks like cursive.)

f-brinkmann

Thanks! I found data that is returned twice. Apart from that its mostly small suggestions.

f-brinkmann · 2025-10-17T07:16:13Z

pyfar/io/io.py

+    # import as spherical coordinates: r, theta, phi
+    if hasattr(ita_coordinates, 'sph'):
+        if ita_coordinates.cart.size == 0:
+            return pf.Coordinates()


See comment above from sikersten. I would suggest to return None here, too.

f-brinkmann · 2025-10-17T07:19:47Z

pyfar/io/io.py

+                data_in, matlab_data.abscissa,
+                comment=comment)
+
+    metadata = _matlab_to_dict(matlab_data)


This return redundant information, e.g., matlab_data.sampling_rate and matlab_data.data that are already returned in the Signal. I would suggest to remove all data that is already returned elsewehre.

Thank you for your review!

I'm not quite sure what exactly I should do.
Are you suggesting including less redundant data in the metadata? For example, removing the matlab_data.domain or matlab_data.sampling_rate from the metadata?

Wouldn't that be too confusing? Because the variable data stores an instance of pf.Signal/pf.TimeData/pf.FrequencyData, and in the variable metadata we only have the raw data for this instance, not the instance itself.

Good point Fabian, since there is not straight forward solution for this, I would still leave it as it is.

Would it make sense to exclude at least the matlab_data.data from metadata? This bascially doubles the RAM requirements and might be annyoing if reading large files.

+1 for @f-brinkmann's suggestion

f-brinkmann · 2025-10-17T07:21:12Z

pyfar/io/io.py

+    object_coordinates : pyfar.Coordinates
+        The object coordinates of the `*.ita` file.
+    channel_coordinates : pyfar.Coordinates
+        The channel coordinates of the `*.ita` file.


In case we follow sikerstens suggestion below, this should become

Suggested change

object_coordinates : pyfar.Coordinates

The object coordinates of the `*.ita` file.

channel_coordinates : pyfar.Coordinates

The channel coordinates of the `*.ita` file.

object_coordinates : pyfar.Coordinates, None

The object coordinates of the `*.ita` file if they exist, ``None`` otherwise.

channel_coordinates : pyfar.Coordinates, None

The channel coordinates of the `*.ita` file if they exist, ``None`` otherwise.

Maybe add a note on the difference of object and channel coordinates? Do we need both? Just asking because I'm not familiar with this.

The ITA documentation in MATLAB contains some brief descriptions:

channelCoordinates: coordinates for each channel (must be itaCoordinates) objectCoordinates: the coordinates of the measured object

Would that be enough for us? I am also not very familiar with the purpose and functionality of channelCoordinates and objectCoordinates, so I would appreciate the opinion of someone who worked with this :)

Same concept as in sofa with listener and receiver.
One object can have multiple channels with relative positioning to the object.

f-brinkmann · 2025-10-17T07:23:25Z

pyfar/io/io.py

+            weights=weights)
+
+    # import as spherical coordinates: r, theta, phi
+    if hasattr(ita_coordinates, 'sph'):


@mberz can you clarify? The files I saw, that contained both had identical data stored in different coordinate conventions, I think.

f-brinkmann · 2025-10-17T07:34:31Z

pyfar/io/io.py

+    mat_dict = {}
+    for fieldname in mat_obj._fieldnames:
+        elem = mat_obj.__dict__[fieldname]
+        if isinstance(elem, spio.matlab.mio5_params.mat_struct):


I think this line causes the deprecation errors and should be changed accordingly:

DeprecationWarning: Please import mat_struct from the scipy.io.matlab namespace; the scipy.io.matlab.mio5_params namespace is deprecated and will be removed in SciPy 2.0.0.

ahms5

nice, thank you!

ahms5 · 2025-10-17T09:43:08Z

pyfar/io/io.py

+    """
+    Read an `*.ita` file created using the ITA-toolbox.
+
+    ITA-Tollbox is a MATLAB Toolbox for Acoustics [#]_.


Suggested change

ITA-Tollbox is a MATLAB Toolbox for Acoustics [#]_.

ITA-Toolbox is a MATLAB Toolbox for Acoustics [#]_.

ahms5 · 2025-10-17T09:46:49Z

pyfar/io/io.py

+
+
+def _ita_to_pyfar_coordinates(ita_coordinates):
+    if hasattr(ita_coordinates, 'weights'):


maybe add a short docstring here.

ahms5 · 2025-10-17T09:48:01Z

pyfar/io/io.py

+            weights=weights)
+
+    # import as spherical coordinates: r, theta, phi
+    if hasattr(ita_coordinates, 'sph'):


I can either be 'cart' or 'sph', similar to what we has before in our coordinates class.

ahms5 · 2025-10-17T09:48:54Z

pyfar/io/io.py

+
+
+def _matlab_to_dict(mat_obj):
+    """Recursively transform MATLAB struct objects to nested dictionaries."""


maybe also add documation about input and output of the method

ahms5 · 2025-10-17T09:51:48Z

tests/test_io_data/read_ita/dirac_itaAudio.ita

maybe move to tests/test_io_data/read_ita/* in this way it would be cleaner and easier to distinguish where it bolongs.

ahms5 · 2025-10-17T09:53:11Z

tests/test_io_read_ita.py

+    assert np.array_equal(metadata_channel_coords_reshaped,
+                            channel_coords.cartesian) and \
+        np.array_equal(channel_coords.cartesian, channelCoordinates)


Suggested change

assert np.array_equal(metadata_channel_coords_reshaped,

channel_coords.cartesian) and \

np.array_equal(channel_coords.cartesian, channelCoordinates)

assert np.array_equal(metadata_channel_coords_reshaped,

channel_coords.cartesian)

assert np.array_equal(channel_coords.cartesian, channelCoordinates)

ahms5 · 2025-10-17T10:00:01Z

tests/test_io_read_ita.py

+import os
+import pytest
+import numpy as np
+


is there any test about the metadata which is not part of other returned data?

Thank you for the review!

Regarding your question: No, not really.
Which data from the metadata should I also test?

In the metadata dictionary there are also keys such as "classname", "classversion", "comment", "history", "fileName", "dateCreated", "dateModified", "dateSaved", "userData", "plotAxesProperties", "plotLineProperties", "userName", "channelNames", "channelUnits", "channelSensors", "channelUserData", "dataTypeOutput", but they are mostly empty in most files we are testing.

mberz and others added 13 commits July 26, 2023 12:02

initial version of read_ita function, function name is up for discussion

9868bbf

Adding ita files for testing ita_read

5d1c9c1

read_ita + tests

63da074

Merge branch 'develop' into read_ita_mni

2ce9e67

itaCoordinates to itaRead(tests missing)

8042c2f

Add test for itaCoordinates

93acd42

refactor ita_read

de116c1

Merge branch 'develop' into read_ita_mni

b76f553

cosmetics and bugfixes

d8e0f6b

Merge branch 'develop' into dev_read_ita

2da8342

discussion

86bea22

Draft version of test functions for read_ita

1a8b2c6

Added two new test functions

5e2ad07

h-chmeruk added duplicate This issue or pull request already exists audio Classes pyfar.classes.audio labels Sep 4, 2025

h-chmeruk commented Sep 4, 2025

View reviewed changes

h-chmeruk self-assigned this Sep 29, 2025

ahms5 mentioned this pull request Oct 8, 2025

Dev read ita #720

Closed

3 tasks

sikersten requested changes Oct 14, 2025

View reviewed changes

Apply suggestions from code review

dc909fd

h-chmeruk marked this pull request as ready for review October 16, 2025 08:59

h-chmeruk requested review from a team, ahms5, f-brinkmann, hoyer-a, mberz, pingelit and sikersten October 16, 2025 11:19

f-brinkmann requested changes Oct 17, 2025

View reviewed changes

ahms5 requested changes Oct 17, 2025

View reviewed changes

h-chmeruk added 2 commits November 11, 2025 15:00

Applied some changes from the reviews

b35e00b

Removed matlab_data.data from metadata

a0e8f60

mberz added this to Weekly Planning Feb 6, 2026

github-project-automation bot moved this to Backlog in Weekly Planning Feb 6, 2026

github-project-automation bot moved this from Backlog to Require review in Weekly Planning Feb 6, 2026

mberz moved this from Require review to Implementation in progress in Weekly Planning Feb 6, 2026



		def read_ita(filename):
		"""Read a *.ita file from the ITA-toolbox.

	The data contained in the *.ita file.
	The audio data contained in the *.ita file.

	"""Read a *.ita file from the ITA-toolbox.
	"""Read an *.ita file created using the ITA-toolbox.

		return data, object_coordinates, channel_coordinates, metadata


		def _to_coordinates(ita_coordinates):

	def _to_coordinates(ita_coordinates):
	def _ita_to_pyfar_coordinates(ita_coordinates):

	ITA-Tollbox is a MATLAB Toolbox for Acoustics [#]_.
	ITA-Toolbox is a MATLAB Toolbox for Acoustics [#]_.



		def _ita_to_pyfar_coordinates(ita_coordinates):
		if hasattr(ita_coordinates, 'weights'):



		def _matlab_to_dict(mat_obj):
		"""Recursively transform MATLAB struct objects to nested dictionaries."""

Conversation

h-chmeruk commented Sep 4, 2025

Which issue(s) will be closed by this pull request?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sikersten left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h-chmeruk commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f-brinkmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h-chmeruk Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahms5 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h-chmeruk commented Oct 16, 2025 •

edited

Loading

h-chmeruk Nov 11, 2025 •

edited

Loading