Skip to content

fix: add missing commas in test_ship.py and fix logger string formatting - #208

Open
Alexx3890 wants to merge 2 commits into
52North:mainfrom
Alexx3890:fix/tier-2-tier-3
Open

Alexx3890 wants to merge 2 commits into
52North:mainfrom
Alexx3890:fix/tier-2-tier-3

Conversation

@Alexx3890

Copy link
Copy Markdown

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.py

    • Fixed broken logger.info() calls in get_dist() by adding %s format specifiers so distance, speed, and time values are correctly included in log output instead of being silently dropped.
  • Modified WeatherRoutingTool/algorithms/isofuel.py

    • Fixed the logger.info() call in determine_timespread() to use %s formatting for delta_time.
  • Modified tests/test_ship.py

    • Added missing commas in the message NumPy array definitions within test_shipparams_get_element and test_shipparams_get_single.
    • This prevents unintended implicit string literal concatenation ('Error' 'OK' → 'ErrorOK') and restores the expected four-element test arrays.

Further Details

Summary

This PR resolves two small, mechanical bugs:

1. Logger %s Formatting Bugs

In isochrone.py and isofuel.py, some logger.info() calls passed values as additional arguments without providing corresponding %s placeholders.

For example:

logger.info('dist=', dist)

and:

logger.info('delta_time', delta_time)

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:

logger.info('dist=%s', dist)

and:

logger.info('delta_time: %s', delta_time)

This ensures that the relevant distance, speed, time, and delta_time values are properly displayed in the logs.

2. Test Array Comma Typo

In tests/test_ship.py, the message arrays in test_shipparams_get_element and test_shipparams_get_single contained:

'Error' 'OK'

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:

['OK', 'OK', 'Error', 'OK']

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.py comma 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:

Copilot AI lite review requested due to automatic review settings September 17, 2026 14:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 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 in get_dist() (isochrone.py) so dist/time/bs values are actually emitted.
  • Fix logger.info() formatting in determine_timespread() (isofuel.py) so delta_time is included in logs.
  • Add missing commas in tests/test_ship.py message 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.

Comment thread WeatherRoutingTool/algorithms/isofuel.py Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants