add coordinates notebook #6
Conversation
ahms5
commented
Feb 25, 2024
•
ahms5
commented
- Move overview of coordinate systems and table to pyfar class documentation.
There was a problem hiding this comment.
Thanks for drafting, I mostly had comments for small improvements. Apart from that
- should we add the table that explains each coordinate, which is currently located here: https://pyfar.readthedocs.io/en/latest/concepts/pyfar.coordinates.html. I would vote yes, if the new notebook replaces the concepts in pyfar.
- I could add a thumbnail showing the cartesian coordinate system (top left in the image). Might be more intuituve than the scatter plot that currently shows up.
| "source": [ | ||
| "import pyfar as pf\n", | ||
| "import numpy as np\n", | ||
| "import matplotlib.pyplot as plt" |
There was a problem hiding this comment.
I would sugest to add %matplotlib inline. In this case you don't have to use plt.show() below.
There was a problem hiding this comment.
nice, thanks, i didn't know.
| "source": [ | ||
| "## Coordinates\n", | ||
| "\n", | ||
| "In acoustics research, we often deal with different coordinate systems when handling sampling points. This can get complicated. That's why we have the Coordinates class in pyfar. The :py:func:`Coordinates class <pyfar.classes.coordinates.Coordinates>` was designed for storing, working with, and accessing coordinate points. You can easily switch between different coordinate systems, rotate points, and do other useful stuff. For example, you can store microphone positions in a spherical microphone array or loudspeaker positions in a sound field synthesis system." |
There was a problem hiding this comment.
The link to the pyfar documentation does not work and should probably be replaced with a URL.
| "source": [ | ||
| "## Supported Coordinate systems\n", | ||
| "\n", | ||
| "Coordinate systems come in different flavors, like cartesian, spherical or cylindrical, where spherical has a couple of subdefinitions. The image below shows the available coordinate systems.\n", |
There was a problem hiding this comment.
Maybe we could already make clear that
- a coodinate system is defined by a set of coordinates, e.g. 'cartesian' consists of x, y, and z.
- each coordinate has a unique name. If a coordinate is contained in multiple coordinate systems (e.g. 'z' in cartesian and cylindrical) this means the values for z are the same in both cases.
| "source": [ | ||
| "### Entering coordinate points\n", | ||
| "\n", | ||
| "You can input coordinate points manually into the system. By default, the system assumes Cartesian coordinates. Internally, all points are consistently stored in the Cartesian coordinate system." |
There was a problem hiding this comment.
Maybe make clear here or by a comment in the code that you are entering x, y, and z-vales.
I think the information that everything is stored in cartesian internally is important. But it might be too early here and I would add that this can cause small rounding errors when entering points in other coordinate systems.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "To initialize `Coordinates` objects for different coordinate systems, you can use specific constructors like `pf.Coordinates.from_spherical_elevation(azimuth, elevation, radius)`. The naming convention for these constructors is always `pf.Coordinates.from_coordinate_system(red, green, blue)`, where `coordinate_system` should be replaced with the desired coordinate system, and `red`, `green`, `blue` describe the order of the coordinate properties. Remember, angles are always defined in radians.\n", |
There was a problem hiding this comment.
You could spcifically link to the figure to mention that this is also in accordance with the colours shown there.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "The cshape of the data is (40,). The term cshape refers to the shape of a single coordinate." |
There was a problem hiding this comment.
Maybe provide a bit more detail here and mention that the coordinates are stored in a numpy array whose last dimension is always 3 and that the cshape ignores this to only show how the coordinate points are organized
There was a problem hiding this comment.
I think, in this way it would be more complex then nessesary. The coords are even saves in 3 separte variables _x, _y, and z. But i think the people must just understand that the cshape is the cshape of any of the coordinates. I added a bit more information to make it more clear.
| "source": [ | ||
| "### Retrieving coordinate points\n", | ||
| "\n", | ||
| "There are different ways to retrieve points from a `Coordinates` object. All points can be obtained in cartesian, spherical, and cylindrical coordinates using the related properties `c.cartesian`, `c.sperical_evaluation` and `c.cylindrical`. Also single properties of each coordinate system convention can be accessed by e.g. `c.azimuth`, `c.radius` or `c.x`. Visit the [coordinate class](https://pyfar.readthedocs.io/en/latest/classes/pyfar.coordinates.html) for more details.\"" |
There was a problem hiding this comment.
| "There are different ways to retrieve points from a `Coordinates` object. All points can be obtained in cartesian, spherical, and cylindrical coordinates using the related properties `c.cartesian`, `c.sperical_evaluation` and `c.cylindrical`. Also single properties of each coordinate system convention can be accessed by e.g. `c.azimuth`, `c.radius` or `c.x`. Visit the [coordinate class](https://pyfar.readthedocs.io/en/latest/classes/pyfar.coordinates.html) for more details.\"" | |
| "There are different ways to retrieve points from a `Coordinates` object. All points can be obtained in cartesian, spherical, and cylindrical coordinates using the related properties `c.cartesian`, `c.sperical_evaluation` and `c.cylindrical`. Also single properties of each coordinate system convention can be accessed by e.g. `c.azimuth`, `c.radius` or `c.x`. Visit the [coordinate class](https://pyfar.readthedocs.io/en/latest/classes/pyfar.coordinates.html) for more details." |
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Of course, we can convert it into other coordinate systems as well. Note that it returns angles always in radiance." |
There was a problem hiding this comment.
Maybe this would be the right place to mention coordinates conversion and underlying cartesian format.
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "azimuth = c.azimuth\n", |
There was a problem hiding this comment.
Why not directly c.azimuth[0] = 0?
| "metadata": {}, | ||
| "source": [ | ||
| "#### Find within\n", | ||
| "Another option is to find all points in a certain area around different points. Different distance measures are available see the [pyfar documentation](https://pyfar.readthedocs.io/en/latest/classes/pyfar.coordinates.html#pyfar.classes.coordinates.Coordinates.find_within) for more information." |
There was a problem hiding this comment.
Maybe mention that this finds everything closer than 3 meters to the query point to more illustrative.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "We can observe that there are 40 points in total over all dimensions. This refers to the attribute `csize`. It corresponds the the `size` of each coordinate.\n" |
There was a problem hiding this comment.
I still find the description of csize, cshape, and cdim a little confusing, how about using:
| "We can observe that there are 40 points in total over all dimensions. This refers to the attribute `csize`. It corresponds the the `size` of each coordinate.\n" | |
| "We can observe that there are 40 coordinate points in the object. This refers to the attribute *coordinate size* or in short `csize`. Note that this is different from the `size` of numpy arrays because the *Coordinates* object stores 3 $\times$ 40 values in total (40 azimuth angles, 40 elevation angles, and 40 radii).\n" |
There was a problem hiding this comment.
Thats not correct, it always stores 40 x, 40y, and 40z values.
There was a problem hiding this comment.
I think the last sententence is rather confusing.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "The `cshape`, `csize`, and `cdim` attributes are similar to numpy's `shape`, `size`, and `dim` of each coordinate.\n" |
There was a problem hiding this comment.
Not sure if we need this explanation.
There was a problem hiding this comment.
Agree. A link to the same logic for audio objects might be sufficient.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "We can also directly access certain coordinates." |
There was a problem hiding this comment.
I just thought that it might be nice to mention our rad2deg converter somewhere
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "azimuth = c.azimuth\n", |
Co-authored-by: Fabian Brinkmann <fabian.brinkmann@mailbox.org>
Co-authored-by: Fabian Brinkmann <fabian.brinkmann@mailbox.org>
Co-authored-by: sikersten <70263411+sikersten@users.noreply.github.com>
| "source": [ | ||
| "import pyfar as pf\n", | ||
| "import numpy as np\n", | ||
| "%matplotlib inline" |
There was a problem hiding this comment.
| "%matplotlib inline" |
There was a problem hiding this comment.
if i dont use it, i need to use plt.show() to show all the plots...
There was a problem hiding this comment.
Is this environment dependent? Works without %matplotlib inline in my case (VScode, python 3.11.7)
There was a problem hiding this comment.
Found this on the web: https://stackoverflow.com/questions/65934740/is-matplotlib-inline-still-needed
Co-authored-by: sikersten <70263411+sikersten@users.noreply.github.com>
Co-authored-by: sikersten <70263411+sikersten@users.noreply.github.com>
| "The image below shows the coordinate systems available with pyfar.\n", | ||
| "\n", | ||
| "\n", | ||
| "Note that each coordinate has a unique name. If a coordinate is contained in multiple coordinate systems (e.g. 'z' in cartesian and cylindrical) this means the values for z are the same in both cases.\n", |
There was a problem hiding this comment.
During the last meeting we agreed to link to the pyfar docs for the table and figures showing the coordinate systems. I'd still vouch for that.
| "cartesian_coordinates = c.cartesian\n", | ||
| "cartesian_coordinates" |
There was a problem hiding this comment.
Why not just?
| "cartesian_coordinates = c.cartesian\n", | |
| "cartesian_coordinates" | |
| "c.cartesian\n", |
Or do you need to access the generated variable later?
| "c.rotate('y', 45)\n", | ||
| "c.show()" |
There was a problem hiding this comment.
Why is the default in degrees here? That's inconsistent with our convention of always using radians.
Should we fix this in pyfar?
| "The image below shows the coordinate systems available with pyfar.\n", | ||
| "\n", |
There was a problem hiding this comment.
I'd link to the pyfar docs on coordinates here to remove redundancy.
Co-authored-by: sikersten <70263411+sikersten@users.noreply.github.com>
