Skip to content

to_windowed_arrays only for non-underway instruments - #361

Merged
j-atkins merged 5 commits into
migrate-v4from
conditional-windowed-arrays
Jul 27, 2026
Merged

to_windowed_arrays only for non-underway instruments#361
j-atkins merged 5 commits into
migrate-v4from
conditional-windowed-arrays

Conversation

@j-atkins

@j-atkins j-atkins commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

This PR adds a check if an instrument is/not an underway instrument (ADCP, UnderwaterST). If not, then use to_windowed_arrays. Otherwise leave as the default NumPy/Dask backed fieldset the fieldset.eval in the underway simulation methods.

All instruments now all have a property method for their InstrumentType (in base.py), which means the is_underway attribute becomes accessible. Related unit tests added for all instruments.

Please note test failures are expected, and will be addressed in root #342 PR

@j-atkins
j-atkins requested a review from erikvansebille July 27, 2026 10:06
Comment thread src/virtualship/instruments/base.py Outdated
Comment thread src/virtualship/instruments/base.py Outdated
Comment thread tests/instruments/test_adcp.py
Co-authored-by: Erik van Sebille <e.vansebille@uu.nl>

@erikvansebille erikvansebille left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good. Happy to be merged when green ticks

@j-atkins

j-atkins commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Looks good. Happy to be merged when green ticks

Great, thanks. Note, certain unit tests are expected to fail (meaning we won't get all green ticks in the PR) but will be addressed in the target #342 branch/PR. They are left to fail for now though to ensure they do not get missed later.

@j-atkins
j-atkins merged commit 1ca6860 into migrate-v4 Jul 27, 2026
8 of 12 checks passed
@j-atkins
j-atkins deleted the conditional-windowed-arrays branch July 27, 2026 11:31
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