Repository: BAMWelDX/weldx

Issue:
weldxfile: catch ASDFWarnings for error handling
Lately I stumbled again into an issue we already had. K3D depends on jupyterlab in its project requirements, but it does not in the current conda-forge release. This downstream issue prevents the loading of our ASDF extension. Unfortunately ASDF decided to catch this exception and just emit a warning in this case, leaving weldx ASDF extension completely broken. A warning will just print something more or less meaningful to stderr and the program just continues.

I suggest that we catch this warning in the WeldxFile class and turn it into an Exception pointing towards the issue.

Discussion:
review-notebook-app[bot]: Check out this pull request on&nbsp; <a href="https://app.reviewnb.com/BAMWelDX/weldx/pull/641"><img align="absmiddle"  alt="ReviewNB" height="28" class="BotMessageButtonImage" src="https://raw.githubusercontent.com/ReviewNB/support/master/images/button_reviewnb.png"/></a> 

 See visual diffs & provide feedback on Jupyter Notebooks. 

---

 <i>Powered by <a href='https://www.reviewnb.com/?utm_source=gh'>ReviewNB</a></i>
codecov[bot]: # [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) Report
> Merging [#641](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) (2dc5b64) into [master](https://codecov.io/gh/BAMWelDX/weldx/commit/8d8601120d74aab1d11b4145c81c47aa0c7eeee3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) (8d86011) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/BAMWelDX/weldx/pull/641/graphs/tree.svg?width=650&height=150&src=pr&token=wdof1qQTsn&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX)](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX)

```diff
@@           Coverage Diff           @@
##           master     #641   +/-   ##
=======================================
  Coverage   94.52%   94.53%           
=======================================
  Files          93       93           
  Lines        5959     5963    +4     
=======================================
+ Hits         5633     5637    +4     
  Misses        326      326           
```


| [Impacted Files](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX) | Coverage Δ | |
|---|---|---|
| [weldx/asdf/file.py](https://codecov.io/gh/BAMWelDX/weldx/pull/641/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX#diff-d2VsZHgvYXNkZi9maWxlLnB5) | `95.58% <100.00%> (+0.07%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX). Last update [8d86011...2dc5b64](https://codecov.io/gh/BAMWelDX/weldx/pull/641?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BAMWelDX).

github-actions[bot]: ## Unit Test Results
       1 files  ±0         1 suites  ±0   1m 53s [:stopwatch:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "duration of all tests") +27s
1 925 tests ±0  1 925 [:heavy_check_mark:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "passed tests") ±0  0 [:zzz:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "skipped / disabled tests") ±0  0 [:x:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "failed tests") ±0 

Results for commit 2dc5b646. ± Comparison against base commit 8d860112.

CagtayFabry: if it works in your local envs that is good enough for me, let's just hope for a new k3d release soon 

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 5fdff2d..19501b8 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -15,13 +15,24 @@ removed
 changes
 =======
 
+- `WeldxFile` now raises an exception, if a warning is emitted during loading the weldx
+  ASDF extension, this should prevent erroneous data during loading, for example
+  missing dependencies. `[#641] <https://github.com/BAMWelDX/weldx/pull/641>`__
+
 -  `WeldxFile` now hides ASDF added fields like history and asdf_library
    from the dictionary interface. To access these, there are separate
    properties `[#625] <https://github.com/BAMWeldX/weldx/pull/625>`__.
 
+-  Allow handling of ``time`` values as singular coordinates without
+   dimensions in some classes `[#635]
+   <https://github.com/BAMWeldX/weldx/pull/635>`__.
+
 fixes
 =====
 
+-  Fix wrong dimension order being passed through in `SpatialData`
+   `[#635] <https://github.com/BAMWeldX/weldx/pull/635>`__.
+
 documentation
 =============
 
diff --git a/tutorials/weldxfile.ipynb b/tutorials/weldxfile.ipynb
index 24a885d..2e3b2ef 100644
--- a/tutorials/weldxfile.ipynb
+++ b/tutorials/weldxfile.ipynb
@@ -541,4 +541,4 @@
  },
  "nbformat": 4,
  "nbformat_minor": 5
-}
+}
\ No newline at end of file
diff --git a/weldx/asdf/file.py b/weldx/asdf/file.py
index f022058..9e06a06 100644
--- a/weldx/asdf/file.py
+++ b/weldx/asdf/file.py
@@ -2,6 +2,8 @@
 import copy
 import io
 import pathlib
+import warnings
+
 from collections.abc import MutableMapping, ValuesView
 from contextlib import contextmanager
 from io import BytesIO, IOBase
@@ -23,6 +25,7 @@ import asdf
 import numpy as np
 from asdf import AsdfFile, generic_io
 from asdf import open as open_asdf
+from asdf.exceptions import AsdfWarning
 from asdf.tags.core import Software
 from asdf.util import FileType, get_file_type
 from jsonschema import ValidationError
@@ -271,22 +274,28 @@ class WeldxFile(_ProtectedViewDict):
             )
 
         # If we have data to write, we do it first, so a WeldxFile is always in sync.
-        if tree or new_file_created:
-            asdf_file = self._write_tree(
-                filename_or_file_like,
-                tree,
-                asdffile_kwargs,
-                write_kwargs,
-                new_file_created,
-            )
-            if isinstance(filename_or_file_like, SupportsFileReadWrite):
-                filename_or_file_like.seek(0)
-        else:
-            asdf_file = open_asdf(
-                filename_or_file_like,
-                mode=self.mode,
-                **asdffile_kwargs,
-            )
+        with warnings.catch_warnings():
+            warnings.filterwarnings(
+                category=AsdfWarning,
+                action="error",
+                message="asdf.extensions plugin from package weldx.*",
+            )  # we turn asdf warnings about loading the weldx extension into an error.
+            if tree or new_file_created:
+                asdf_file = self._write_tree(
+                    filename_or_file_like,
+                    tree,
+                    asdffile_kwargs,
+                    write_kwargs,
+                    new_file_created,
+                )
+                if isinstance(filename_or_file_like, SupportsFileReadWrite):
+                    filename_or_file_like.seek(0)
+            else:
+                asdf_file = open_asdf(
+                    filename_or_file_like,
+                    mode=self.mode,
+                    **asdffile_kwargs,
+                )
         self._asdf_handle: AsdfFile = asdf_file
 
         # initialize protected key interface.
diff --git a/weldx/geometry.py b/weldx/geometry.py
index 8a4cf40..cac96cc 100644
--- a/weldx/geometry.py
+++ b/weldx/geometry.py
@@ -2538,6 +2538,9 @@ class SpatialData:
                 add_dims=["n"],
             )
 
+        # make sure we have correct dimension order
+        self.coordinates = self.coordinates.transpose(..., "n", "c")
+
         if self.triangles is not None:
             if not isinstance(self.triangles, np.ndarray):
                 self.triangles = np.array(self.triangles, dtype="uint")
diff --git a/weldx/tags/core/transformations/local_coordinate_system.py b/weldx/tags/core/transformations/local_coordinate_system.py
index 95ed41f..76ab1d0 100644
--- a/weldx/tags/core/transformations/local_coordinate_system.py
+++ b/weldx/tags/core/transformations/local_coordinate_system.py
@@ -40,7 +40,7 @@ class LocalCoordinateSystemConverter(WeldxConverter):
             #         ctx.set_array_storage(coordinates.data, "inline")
             tree["coordinates"] = coordinates
 
-        if "time" in obj.dataset.dims:
+        if "time" in obj.dataset.coords:
             tree["time"] = obj.time.as_timedelta_index()
 
         if obj.reference_time is not None:
diff --git a/weldx/time.py b/weldx/time.py
index 1f6f209..a651f3f 100644
--- a/weldx/time.py
+++ b/weldx/time.py
@@ -668,7 +668,10 @@ class Time:
         if "time" in time.coords:
             time = time.time
         time_ref = time.weldx.time_ref
-        time_index = pd.Index(time.values)
+        if time.shape:
+            time_index = pd.Index(time.values)
+        else:
+            time_index = pd.Index([time.values])
         if time_ref is not None:
             time_index = time_index + time_ref
         return time_index
diff --git a/weldx/transformations/cs_manager.py b/weldx/transformations/cs_manager.py
index 0d1b82b..cf3691d 100644
--- a/weldx/transformations/cs_manager.py
+++ b/weldx/transformations/cs_manager.py
@@ -1697,7 +1697,7 @@ class CoordinateSystemManager:
         if not isinstance(data, xr.DataArray):
             data = xr.DataArray(data, dims=["n", "c"], coords={"c": ["x", "y", "z"]})
 
-        time = data.time if "time" in data.dims else None
+        time = data.time if "time" in data.coords else None
         lcs = self.get_cs(
             source_coordinate_system_name, target_coordinate_system_name, time=time
         )
diff --git a/weldx/transformations/local_cs.py b/weldx/transformations/local_cs.py
index 112085a..769ac4e 100644
--- a/weldx/transformations/local_cs.py
+++ b/weldx/transformations/local_cs.py
@@ -78,6 +78,7 @@ class LocalCoordinateSystem(TimeDependent):
         # warn about dropped time data
         if (
             time is not None
+            and len(Time(time)) > 1
             and "time" not in orientation.dims
             and (isinstance(coordinates, TimeSeries) or "time" not in coordinates.dims)
         ):
@@ -548,7 +549,7 @@ class LocalCoordinateSystem(TimeDependent):
             `True` if the coordinate system is time dependent, `False` otherwise.
 
         """
-        return self.time is not None or self._coord_ts is not None
+        return self._coord_ts is not None or ("time" in self._dataset.dims)
 
     @property
     def has_timeseries(self) -> bool:
@@ -593,7 +594,7 @@ class LocalCoordinateSystem(TimeDependent):
             Time-like data array representing the time union of the LCS
 
         """
-        if "time" in self._dataset.dims:
+        if "time" in self._dataset.coords:
             return Time(self._dataset.time, self.reference_time)
         return None
 
@@ -667,9 +668,9 @@ class LocalCoordinateSystem(TimeDependent):
         if "time" not in self.orientation.dims:  # don't interpolate static
             return self.orientation
         if time.max() <= self.time.min():  # only use edge timestamp
-            return self.orientation.values[0]
+            return self.orientation.isel(time=0).data
         if time.min() >= self.time.max():  # only use edge timestamp
-            return self.orientation.values[-1]
+            return self.orientation.isel(time=-1).data
         # full interpolation with overlapping times
         return ut.xr_interp_orientation_in_time(self.orientation, time)
 
@@ -686,9 +687,9 @@ class LocalCoordinateSystem(TimeDependent):
         if "time" not in self.coordinates.dims:  # don't interpolate static
             return self.coordinates
         if time.max() <= self.time.min():  # only use edge timestamp
-            return self.coordinates[0]
+            return self.coordinates.isel(time=0).data
         if time.min() >= self.time.max():  # only use edge timestamp
-            return self.coordinates[-1]
+            return self.coordinates.isel(time=-1).data
         # full interpolation with overlapping times
         return ut.xr_interp_coordinates_in_time(self.coordinates, time)
 
diff --git a/weldx/util/xarray.py b/weldx/util/xarray.py
index b821a53..139f135 100644
--- a/weldx/util/xarray.py
+++ b/weldx/util/xarray.py
@@ -677,7 +677,7 @@ def xr_interp_coordinates_in_time(
         Interpolated data
 
     """
-    if "time" not in da.dims:
+    if "time" not in da.dims:  # not time dependent
         return da
 
     times = Time(times).as_pandas_index()
@@ -734,7 +734,7 @@ class WeldxAccessor:
     def time_ref_restore(self) -> xr.DataArray:
         """Convert DatetimeIndex back to TimedeltaIndex + reference Timestamp."""
         da = self._obj.copy()
-        if "time" not in da.dims:
+        if "time" not in da.coords:
             return da
 
         if is_datetime64_dtype(da.time):
@@ -757,7 +757,7 @@ class WeldxAccessor:
     def time_ref(self) -> Union[pd.Timestamp, None]:
         """Get the time_ref value or `None` if not set."""
         da = self._obj
-        if "time" in da.dims and "time_ref" in da.time.attrs:
+        if "time" in da.coords and "time_ref" in da.time.attrs:
             return da.time.attrs["time_ref"]
 
         return None


Test patch:
diff --git a/weldx/tests/transformations/test_cs_manager.py b/weldx/tests/transformations/test_cs_manager.py
index e6af3c6..b89234b 100644
--- a/weldx/tests/transformations/test_cs_manager.py
+++ b/weldx/tests/transformations/test_cs_manager.py
@@ -2614,6 +2614,7 @@ def test_issue_289_interp_outside_time_range(
     exp_orient = WXRotation.from_euler("x", exp_angle, degrees=True).as_matrix()
     exp_coords = [0, 0, 0] if time_dep_coords and all_less else [1, 1, 1]
 
+    assert cs_br.is_time_dependent is False
     assert cs_br.time is None
     assert cs_br.coordinates.values.shape == (3,)
     assert cs_br.orientation.values.shape == (3, 3)
diff --git a/weldx/tests/transformations/test_local_cs.py b/weldx/tests/transformations/test_local_cs.py
index 60026b5..9cf6fb3 100644
--- a/weldx/tests/transformations/test_local_cs.py
+++ b/weldx/tests/transformations/test_local_cs.py
@@ -497,6 +497,7 @@ def test_issue_289_interp_outside_time_range(
     exp_orient = WXRotation.from_euler("x", exp_angle, degrees=True).as_matrix()
     exp_coords = [0, 0, 0] if time_dep_coords and all_less else [1, 1, 1]
 
+    assert lcs_interp.is_time_dependent is False
     assert lcs_interp.time is None
     assert lcs_interp.coordinates.values.shape == (3,)
     assert lcs_interp.orientation.values.shape == (3, 3)
@@ -518,7 +519,8 @@ def test_interp_time_discrete_single_time():
     exp_orient = WXRotation.from_euler("x", 90, degrees=True).as_matrix()
 
     lcs_interp = lcs.interp_time("2s")
-    assert lcs_interp.time is None
+    assert lcs_interp.is_time_dependent is False
+    assert lcs_interp.time.equals(Time("2s"))
     assert lcs_interp.coordinates.values.shape == (3,)
     assert lcs_interp.orientation.values.shape == (3, 3)
     assert np.all(lcs_interp.coordinates.data == exp_coords)