search

NOAA-OWP / t-route

Tree based hydrologic and hydraulic routing
Other
44 stars 50 forks source link

Configuring `t-route` with `output_parameters.parquet_output` without `prefix_ids` causes crash #820

Closed aaraney closed 3 months ago

aaraney commented 3 months ago

https://github.com/NOAA-OWP/t-route/blob/5865910b73865bedf281cd79d7de4463675a2834/src/troute-nwm/src/nwm_routing/output.py#L503

This will crash if prefix_ids was not specified in the configuration.

Traceback (most recent call last):
  File "/python/versions/3.10.10/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/python/versions/3.10.10/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/__main__.py", line 2223, in <module>
    main_v04(v_args[1])
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/__main__.py", line 304, in main_v04
    nwm_output_generator(
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/output.py", line 504, in nwm_output_generator
    timeseries_df = _parquet_output_format_converter(flowveldepth, restart_parameters.get("start_datetime"), dt,
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/output.py", line 73, in _parquet_output_format_converter
    location_ids = prefix_ids + '-' + df['location_id'].astype(str)
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

https://github.com/NOAA-OWP/t-route/blob/5865910b73865bedf281cd79d7de4463675a2834/src/troute-config/troute/config/output_parameters.py#L57

I think the above should not be optional either it should be required field, or have default (e.g. wb-). @shorvath-noaa does that sound right? Happy to submit at PR

shorvath-noaa commented 3 months ago

https://github.com/NOAA-OWP/t-route/blob/5865910b73865bedf281cd79d7de4463675a2834/src/troute-nwm/src/nwm_routing/output.py#L503

This will crash if prefix_ids was not specified in the configuration.

Traceback (most recent call last):
  File "/python/versions/3.10.10/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/python/versions/3.10.10/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/__main__.py", line 2223, in <module>
    main_v04(v_args[1])
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/__main__.py", line 304, in main_v04
    nwm_output_generator(
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/output.py", line 504, in nwm_output_generator
    timeseries_df = _parquet_output_format_converter(flowveldepth, restart_parameters.get("start_datetime"), dt,
  File "/github/t-route/t/venv/lib/python3.10/site-packages/nwm_routing/output.py", line 73, in _parquet_output_format_converter
    location_ids = prefix_ids + '-' + df['location_id'].astype(str)
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'

https://github.com/NOAA-OWP/t-route/blob/5865910b73865bedf281cd79d7de4463675a2834/src/troute-config/troute/config/output_parameters.py#L57

I think the above should not be optional either it should be required field, or have default (e.g. wb-). @shorvath-noaa does that sound right? Happy to submit at PR

Correct, the default should be "wb-". The other parameters should probably have better defaults too. I'll make a quick PR.

aaraney commented 3 months ago

Awesome, thanks @shorvath-noaa!

shorvath-noaa commented 3 months ago

Resolved with #821