[amanbn8]

Related Issue / Discussion:

Fixes Issue #213

Changes:

• Modified WeatherRoutingTool/ship/ship.py: Boat.get_weather_data() now checks self.weather_data is None instead of == None.
• Modified tests/test_ship.py: new test test_get_weather_data_opens_file_only_once that wraps xr.open_dataset with a counter, calls get_weather_data() twice on a ConstantFuelBoat, and checks that the file is opened once and the same Dataset object is returned both times.

Further Details:

Summary:

get_weather_data() is documented as opening the weather NetCDF once and caching it. The guard compared an xarray.Dataset to None with ==, which xarray evaluates element-wise and returns a Dataset that is truthy. So after the first call the condition stayed true and the file was reopened and fully loaded on every call, from every routing step, for both the isobased and genetic algorithms.

Before: xr.open_dataset is called once per get_weather_data() call, and each call returns a new Dataset.

After: the file is opened once per Boat instance and the cached Dataset is returned on later calls, which is what the docstring already describes.

The new test fails on main (two calls to open_dataset, two different objects) and passes with the fix. flake8 runs clean on both files with the repo's settings.

Dependencies:

None.

PR Checklist:

In the context of this PR, I:

• have (already previously) filled the 52North Contributor License Agreement and received positive feedback on this matter
• have filled the 52North Contributor License Agreement and am waiting for feedback
• provide unit tests embedded in the WRT test framework (WeatherRoutingTool/tests) that allow the simple testing of the new/modified functionalities. All (previous and new) unit tests execute without new error messages.
• ensure that the code formatter runs without errors/warnings
• ensure that my changes follow the WRT’s guidelines for contributing at the time of the contribution

[kdemmich]

Dear user,
thank you for your interest in contributing to our WRT! I would like to point you to our guidelines for software contributions which require to follow 52 North's CLA guidelines for all PRs and, in particular, fill the 52°North Contributor License Agreement. As long as this has not been filled, I can not assign you to any issues or merge any PRs. Please don't hesitate to contact me in case of related questions.

[amanbn8]

Hi @kdemmich, thanks for the pointer and for taking a look.

I read through the CLA guidelines, and as far as I can tell the agreement has to be printed, signed and sent by post to 52°North in Germany. I'm a student in India and that's unfortunately not really feasible for me, so I won't be able to complete the CLA in its current form. If an electronically signed scan sent by email would be acceptable, let me know and I'll send one right away.

Otherwise, feel free to close this PR. Thanks again!

[kdemmich]

Dear @amanbn8,
thanks a lot for your enthusiasm in contributing to our routing tool. As stated in the CLA guidelines, the printed copies of the CLA are required. I will thus close your PR.