Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Python 3.12 and Migrate to Xarray DataTree #1419

Merged
merged 30 commits into from
Jan 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fa0fec6
initial commit to support python 3.12 and migrate to xarray DataTree
ctuguinay Dec 26, 2024
eb3ba8c
remove additional py3.11 file
ctuguinay Dec 26, 2024
bf77e4d
update imports
ctuguinay Dec 26, 2024
2c654f1
add todo note to search through pr history to learn how to correctly …
ctuguinay Dec 26, 2024
6f0a68e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 26, 2024
08a254a
fix data tree render import
ctuguinay Dec 27, 2024
4482e5a
pin xarray to later 2024 november version
ctuguinay Dec 27, 2024
5246726
bump minimum python testing version and supported version to 3.10
ctuguinay Dec 27, 2024
0b3770f
set time3 in nmea to time_nmea
ctuguinay Dec 27, 2024
b565593
resolve merge conflicts
ctuguinay Dec 30, 2024
a9c2650
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 30, 2024
2959453
use channel all for ek80 sonar group
ctuguinay Dec 30, 2024
5c967cf
add support for nmea_time
ctuguinay Dec 30, 2024
3dbe83b
import xarray import open_datatree test change
ctuguinay Dec 30, 2024
d732da8
fix datatree import pt 2
ctuguinay Dec 30, 2024
3c63200
support 3.12 in setup cfg
ctuguinay Dec 30, 2024
d75a1f3
update dim check
ctuguinay Dec 30, 2024
5f0cedd
resolve merge conflict
ctuguinay Dec 30, 2024
1c4eb28
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 30, 2024
6e63347
remove test notebook
ctuguinay Dec 30, 2024
0677b86
fix import
ctuguinay Dec 30, 2024
8a3380a
comment out TestEchoData and save it for issue 1420
ctuguinay Dec 30, 2024
b19aa35
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Dec 30, 2024
592a46f
Merge branch 'modify_max_python' into support_python_312
ctuguinay Dec 30, 2024
1173bd7
set max python to <= 3.12
ctuguinay Dec 30, 2024
1fb02bc
Merge branch 'modify_max_python' into support_python_312
ctuguinay Dec 30, 2024
a86333e
set to less than ython 3.13
ctuguinay Dec 30, 2024
4999e7f
set python 3.12 in setup cfg
ctuguinay Dec 30, 2024
281f146
add ek80 channel test with two beam groups
ctuguinay Dec 31, 2024
b1f147f
remove unnecessary comment
ctuguinay Dec 31, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci_helpers/py3.9.yaml → .ci_helpers/py3.12.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: echopype
channels:
- conda-forge
dependencies:
- python=3.9
- python=3.12
- pip
- pip:
- -r ../requirements.txt
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["3.9", "3.10", "3.11"]
python-version: ["3.10", "3.11", "3.12"]
runs-on: [ubuntu-latest]
experimental: [false]
services:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ["3.9", "3.10", "3.11"]
python-version: ["3.10", "3.11", "3.12"]
runs-on: [ubuntu-latest]
experimental: [false]
services:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows-utils.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
fail-fast: false
matrix:
include:
- python-version: 3.9
- python-version: 3.12
experimental: false
defaults:
run:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
fail-fast: false
matrix:
include:
- python-version: 3.9
- python-version: 3.12
experimental: false
defaults:
run:
Expand Down
2 changes: 1 addition & 1 deletion docs/source/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ To create an environment for developing Echopype, we recommend the following ste
```shell
# Create a conda environment using the supplied requirements files
# Note the last one docs/requirements.txt is only required for building docs
conda create -c conda-forge -n echopype --yes python=3.9 --file requirements.txt --file requirements-dev.txt --file docs/requirements.txt
conda create -c conda-forge -n echopype --yes python=3.12 --file requirements.txt --file requirements-dev.txt --file docs/requirements.txt

# Switch to the newly built environment
conda activate echopype
Expand Down
6 changes: 1 addition & 5 deletions docs/source/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Installation

Echopype is available and tested for Python 3.9-3.11. The latest release can be installed through conda (or mamba, see below) via the [conda-forge channel](https://anaconda.org/conda-forge/echopype):
Echopype is available and tested for Python 3.10-3.12. The latest release can be installed through conda (or mamba, see below) via the [conda-forge channel](https://anaconda.org/conda-forge/echopype):
```shell
# Install via conda-forge
$ conda install -c conda-forge echopype
Expand All @@ -14,10 +14,6 @@ It is available via [PyPI](https://pypi.org/project/echopype):
$ pip install echopype
```

:::{note}
We are working on adding support for Python 3.12 soon!
:::

:::{attention}
It's common to encounter the situation that installing packages using Conda is slow or fails,
because Conda is unable to resolve dependencies.
Expand Down
2 changes: 1 addition & 1 deletion echopype/convert/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from typing import TYPE_CHECKING, Dict, Literal, Optional, Tuple, Union

import fsspec
from datatree import DataTree
from xarray import DataTree

# fmt: off
# black and isort have conflicting ideas about how this should be formatted
Expand Down
6 changes: 3 additions & 3 deletions echopype/convert/set_groups_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,14 @@ def set_nmea(self) -> xr.Dataset:
ds = xr.Dataset(
{
"NMEA_datagram": (
["time1"],
["nmea_time"],
raw_nmea,
{"long_name": "NMEA datagram"},
)
},
coords={
"time1": (
["time1"],
"nmea_time": (
["nmea_time"],
Comment on lines 151 to +159
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about time_nmea to be consistent with the noun-modifier pattern that's used elsewhere?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh just saw that there's ping_time that deviates from this pattern, and it would be odd to make it time_ping. Personally I feel it is cleaner for the other ones to be all like time_X...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh huh I never thought about it that way. I personally see them both as a standalone/compound noun since 'ping' + 'time' refer to different things but together form this one distinct thing and 'ping time' could be operated on like "short + 'ping time'", and 'nmea time' I feel could be operated on similarly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see what you mean, so ping time is when a ping is issues, and nmea is when an nmea sentence is issues. I agree with that. Perhaps we can just keep it this way here. We will at some point need a larger breaking change to keep up with convention changes once more feedback from the community is discussed, but that'll be at least a few months down the road.

time,
{
"axis": "T",
Expand Down
12 changes: 6 additions & 6 deletions echopype/convert/set_groups_ek80.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ def set_sonar(self, beam_group_type: list = ["power", None]) -> xr.Dataset:
# Create dataset
sonar_vars = {
"frequency_nominal": (
["channel"],
["channel_all"],
var["transducer_frequency"],
{
"units": "Hz",
Expand All @@ -237,21 +237,21 @@ def set_sonar(self, beam_group_type: list = ["power", None]) -> xr.Dataset:
},
),
"transceiver_serial_number": (
["channel"],
["channel_all"],
var["serial_number"],
{
"long_name": "Transceiver serial number",
},
),
"transducer_name": (
["channel"],
["channel_all"],
var["transducer_name"],
{
"long_name": "Transducer name",
},
),
"transducer_serial_number": (
["channel"],
["channel_all"],
var["transducer_serial_number"],
{
"long_name": "Transducer serial number",
Expand All @@ -261,8 +261,8 @@ def set_sonar(self, beam_group_type: list = ["power", None]) -> xr.Dataset:
ds = xr.Dataset(
{**sonar_vars, **beam_groups_vars},
coords={
"channel": (
["channel"],
"channel_all": (
["channel_all"],
self.sorted_channel["all"],
self._varattrs["beam_coord_default"]["channel"],
),
Expand Down
4 changes: 2 additions & 2 deletions echopype/echodata/combine.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import numpy as np
import pandas as pd
import xarray as xr
from datatree import DataTree
from xarray import DataTree

from ..utils.io import validate_output_path
from ..utils.log import _init_logger
Expand All @@ -18,7 +18,7 @@

logger = _init_logger(__name__)

POSSIBLE_TIME_DIMS = {"time1", "time2", "time3", "time4", "ping_time"}
POSSIBLE_TIME_DIMS = {"time1", "time2", "time3", "time4", "nmea_time", "ping_time"}
APPEND_DIMS = {"filenames"}.union(POSSIBLE_TIME_DIMS)
DATE_CREATED_ATTR = "date_created"
CONVERSION_TIME_ATTR = "conversion_time"
Expand Down
10 changes: 5 additions & 5 deletions echopype/echodata/echodata.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import fsspec
import numpy as np
import xarray as xr
from datatree import DataTree, open_datatree
from xarray import DataTree, open_datatree
from zarr.errors import GroupNotFoundError, PathNotFoundError

if TYPE_CHECKING:
Expand Down Expand Up @@ -242,7 +242,7 @@ def group_paths(self) -> Set[str]:
def __get_dataset(node: DataTree) -> Optional[xr.Dataset]:
if node.has_data or node.has_attrs:
# validate and clean dtypes
return sanitize_dtypes(node.ds)
return sanitize_dtypes(node.to_dataset())
return None
leewujung marked this conversation as resolved.
Show resolved Hide resolved

def __get_node(self, key: Optional[str]) -> DataTree:
Expand All @@ -265,7 +265,7 @@ def __setitem__(self, __key: Optional[str], __newvalue: Any) -> Optional[xr.Data
if self._tree:
try:
node = self.__get_node(__key)
node.ds = __newvalue
node.dataset = __newvalue
return self.__get_dataset(node)
except KeyError:
raise GroupNotFoundError(__key)
Expand All @@ -283,9 +283,9 @@ def __setattr__(self, __name: str, __value: Any) -> None:
group_path = group["ep_group"]
if self._tree:
if __name == "top":
self._tree.ds = __value
self._tree.dataset = __value
else:
self._tree[group_path].ds = __value
self._tree[group_path].dataset = __value
super().__setattr__(__name, attr_value)

@add_processing_level("L1A")
Expand Down
6 changes: 3 additions & 3 deletions echopype/echodata/widgets/utils.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import uuid
from hashlib import md5

from datatree import DataTree
from datatree.render import RenderTree
from xarray import DataTree
from xarray.core.datatree_render import RenderDataTree

from ..convention.utils import _get_sonar_groups

Expand Down Expand Up @@ -63,7 +63,7 @@ def _single_node_repr(node: DataTree) -> str:


def tree_repr(tree: DataTree) -> str:
renderer = RenderTree(tree)
renderer = RenderDataTree(tree)
lines = []
for pre, _, node in renderer:
if node.has_data or node.has_attrs:
Expand Down
2 changes: 1 addition & 1 deletion echopype/tests/convert/test_convert_ek60.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_convert_ek60_different_num_channel_mode_values(file_path, ek60_path):
np.float32
)

@pytest.mark.test

@pytest.mark.integration
def test_converting_ek60_raw_with_missing_channel_power():
"""
Expand Down
25 changes: 23 additions & 2 deletions echopype/tests/convert/test_convert_ek80.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,6 @@ def test_convert_ek80_mru1(ek80_path):
np.all(echodata["Platform"]["vertical_offset"].data == np.array(parser.mru1["heave"]))
np.all(echodata["Platform"]["heading"].data == np.array(parser.mru1["heading"]))


@pytest.mark.unit
def test_skip_ec150(ek80_path):
"""Make sure we skip EC150 datagrams correctly."""
Expand All @@ -457,7 +456,7 @@ def test_skip_ec150(ek80_path):
assert "backscatter_i" in echodata["Sonar/Beam_group1"].data_vars
assert (
echodata["Sonar/Beam_group1"].dims
== {'channel': 1, 'ping_time': 2, 'range_sample': 115352, 'beam': 4}
== {'channel_all': 1, 'beam_group': 1, 'channel': 1, 'ping_time': 2, 'range_sample': 115352, 'beam': 4}
)


Expand Down Expand Up @@ -531,3 +530,25 @@ def test_parse_ek80_with_invalid_env_datagrams():
env_var = ed["Environment"][var]
assert env_var.notnull().all() and env_var.dtype == np.float64


@pytest.mark.unit
def test_ek80_sonar_all_channel():
"""
Checks that when an EK80 raw file has 2 beam groups, the converted Echodata object contains
all channels in the 'channel_all' dimension of the Sonar group.
"""
# Convert EK80 Raw File
ed = open_raw(
raw_file="echopype/test_data/ek80_new/echopype-test-D20211004-T235714.raw",
sonar_model="EK80"
)

# Grab channels from Sonar group
channel_set = set(ed["Sonar"]["channel_all"].data)

# Grab channels from both beam groups
target_channel_set = set(ed["Sonar/Beam_group1"]["channel"].data)
target_channel_set.update(set(ed["Sonar/Beam_group2"]["channel"].data))

# Check that channel sets are equal
assert channel_set == target_channel_set
2 changes: 1 addition & 1 deletion echopype/tests/convert/test_convert_source_target_locs.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import fsspec
import xarray as xr
import pytest
from datatree import open_datatree
from xarray import open_datatree
from tempfile import TemporaryDirectory
from echopype import open_raw
from echopype.utils.coding import DEFAULT_ENCODINGS
Expand Down
Loading
Loading