Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, targeted fixes aligned with the issue/description, with only an optional logging-style improvement noted.
Pull request overview
This PR addresses Issue #162 by fixing Python logging calls that were dropping/triggering formatting errors due to missing %s placeholders, and by correcting a subtle test-data bug caused by implicit string literal concatenation in tests/test_ship.py.
Changes:
- Fix
logger.info()argument formatting inget_dist()(isochrone.py) so dist/time/bs values are actually emitted. - Fix
logger.info()formatting indetermine_timespread()(isofuel.py) sodelta_timeis included in logs. - Add missing commas in
tests/test_ship.pymessage arrays to prevent'Error' 'OK'from becoming'ErrorOK'.
File summaries
| File | Description |
|---|---|
| WeatherRoutingTool/algorithms/isofuel.py | Corrects logger.info() formatting for delta_time (and keeps time-spread logging adjacent). |
| WeatherRoutingTool/algorithms/isochrone.py | Corrects logger.info() formatting so computed distance/time inputs are logged correctly. |
| tests/test_ship.py | Fixes missing commas in test message arrays to prevent implicit string concatenation. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Dear user, |
Related Issue / Discussion
Fixes Issue #162.
Relates to open PRs #163 and #182 (see details below).
Changes
Please list the central functionalities that have been changed:
Modified
WeatherRoutingTool/algorithms/isochrone.pylogger.info()calls inget_dist()by adding%sformat specifiers so distance, speed, and time values are correctly included in log output instead of being silently dropped.Modified
WeatherRoutingTool/algorithms/isofuel.pylogger.info()call indetermine_timespread()to use%sformatting fordelta_time.Modified
tests/test_ship.pymessageNumPy array definitions withintest_shipparams_get_elementandtest_shipparams_get_single.'Error' 'OK'→'ErrorOK') and restores the expected four-element test arrays.Further Details
Summary
This PR resolves two small, mechanical bugs:
1. Logger
%sFormatting BugsIn
isochrone.pyandisofuel.py, somelogger.info()calls passed values as additional arguments without providing corresponding%splaceholders.For example:
and:
Python's standard logging module uses
%-style formatting for additional arguments. Without a corresponding format specifier, the supplied values are not included in the resulting log message.These calls have been corrected to:
and:
This ensures that the relevant distance, speed, time, and
delta_timevalues are properly displayed in the logs.2. Test Array Comma Typo
In
tests/test_ship.py, themessagearrays intest_shipparams_get_elementandtest_shipparams_get_singlecontained:without a separating comma.
Python implicitly concatenates adjacent string literals, resulting in:
'ErrorOK'instead of two separate elements.
This caused the array to contain only three elements instead of the expected four. Adding the missing comma restores the intended array:
and fixes the corresponding test input.
Note on Overlapping PRs #163 and #182
I am aware that open PRs #163 and #182 also address the
test_ship.pycomma typo. Both appear to be currently inactive.This PR combines that existing test-data fix with the logger formatting corrections related to Issue #162.
If maintainers prefer to merge #163 or #182 for the test fix separately, I am happy to rebase or isolate the logger-related changes into an independent PR.
Dependencies
None (no new dependencies required).
Testing
The affected test suite was run to verify the changes. The test-data correction ensures that the existing ship parameter tests operate on the intended four-element input arrays.
PR Checklist
In the context of this PR, I: