maxmahlke / rocks

for space-rocks :new_moon:
https://rocks.readthedocs.io/en/latest/
MIT License
16 stars 3 forks source link

Catch if ssocard contains NULL character instead of null value #32

Open Siltala opened 7 months ago

Siltala commented 7 months ago

Hi Max,

Since the SsODNet update that came out on Friday, i see rocks 1.9.7 failing for some cases where in other cases it works - i,e, I think this is a regression caused by some SsODNet, rather than rocks change. Example broken use case:

$ rocks albedos 2011 AA37
Traceback (most recent call last):
  File "/home/ubuntu/.local/bin/rocks", line 8, in <module>
    sys.exit(cli_rocks())
  File "/home/ubuntu/.local/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/click/core.py", line 1682, in invoke
    cmd_name, cmd, args = self.resolve_command(ctx, args)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/click/core.py", line 1729, in resolve_command
    cmd = self.get_command(ctx, cmd_name)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/cli.py", line 47, in get_command
    return echo()
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/cli.py", line 384, in echo
    rock = core.Rock(id_, datacloud=catalogues)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 1054, in __init__
    super().__init__(**ssocard)  # type: ignore
  File "/home/ubuntu/.local/lib/python3.10/site-packages/pydantic/main.py", line 171, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 185, in _compute_mean_error
    [np.abs(values["error"][which]) for which in ["min", "max"]]
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 185, in <listcomp>
    [np.abs(values["error"][which]) for which in ["min", "max"]]
numpy.core._exceptions._UFuncNoLoopError: ufunc 'absolute' did not contain a loop with signature matching types <class 'numpy.dtypes.StrDType'> -> None

rocks albedos Ceres, on the other hand, works as expected.

Siltala commented 7 months ago

Actually just running Rock("2011 AA37") is enough to reproduce:

>>> from rocks import Rock
>>> Rock("2011 AA37")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 1054, in __init__
    super().__init__(**ssocard)  # type: ignore
  File "/home/ubuntu/.local/lib/python3.10/site-packages/pydantic/main.py", line 171, in __init__
    self.__pydantic_validator__.validate_python(data, self_instance=self)
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 185, in _compute_mean_error
    [np.abs(values["error"][which]) for which in ["min", "max"]]
  File "/home/ubuntu/.local/lib/python3.10/site-packages/rocks/core.py", line 185, in <listcomp>
    [np.abs(values["error"][which]) for which in ["min", "max"]]
numpy.core._exceptions._UFuncNoLoopError: ufunc 'absolute' did not contain a loop with signature matching types <class 'numpy.dtypes.StrDType'> -> None
Siltala commented 7 months ago

I added a simple print(values) call to line 182 in core.py. It shows that when the crash occurs, values is {'value': 'NULL', 'error': {'min': 'NULL', 'max': 'NULL'}}. I assume these NULL strings are a new SsODNet change and causes the issue?

maxmahlke commented 7 months ago

Hi Lauri, thanks for letting us know, it's indeed the NULL values in the ssoCards that rocks does not like. They should not be there and will be removed in the next release. This may happen every now and then, so I will add a catch for this error in general to the ssoCard ingestion in rocks. I'll get on this asap and will let you know.

Siltala commented 6 months ago

I saw that here was a SsODNet update yesterday with this issue fixed - thanks! I confirm that rocks works as expected now, but I'll leave the issue open since you mention adding a catch in case this occurs again.