Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seperate instruments #35

Merged
merged 40 commits into from
Jun 5, 2024
Merged

Seperate instruments #35

merged 40 commits into from
Jun 5, 2024

Conversation

surgura
Copy link
Collaborator

@surgura surgura commented Jun 4, 2024

  • Move all instruments out of sailship in to their own module
  • Refactor most of sailship
    -> tests for instruments are incomplete and their actual results are most likely incorrect. This will be the next step.

@surgura surgura requested review from erikvansebille and ammedd June 4, 2024 09:18
@ammedd
Copy link
Collaborator

ammedd commented Jun 4, 2024

Anything specific you want feedback on?
It feels hard to review tests that are incomplete and likely incorrect.
In general the structure looks good, though little things like that the different particle sets (.e.g _ADCPParticle) are written with an underscore (although I understand from a programming viewpoint) bug me.

Copy link
Member

@erikvansebille erikvansebille left a comment

Choose a reason for hiding this comment

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

Changes look good to me! A few comments and suggestions below

@surgura surgura requested a review from erikvansebille June 5, 2024 14:48
@surgura surgura merged commit fcb36d7 into development Jun 5, 2024
12 checks passed
@surgura surgura deleted the seperate_instruments branch June 5, 2024 15:12
surgura added a commit that referenced this pull request Aug 18, 2024
* wip drifters

* drifters instrument. just missing test

* drifter test

* Remove old drifter file and fix errors

* codetools

* outputdf wherever i forgot to use it

* added todo list

* ctd instrument

* CTDtest

* use new ctd instrument in sailship

* fix typo

* codetools

* use new parcels api for particle class

* add ctd to init

* comments

* first try adcp

* adcp test

* use adcp instrument in sailship

* cleanup

* instrument ship s t

* use ship st in sailship

* cleanup

* refactor sailship ctd cast

* cleanup

* large cleanup

* add checks that all drifter and argo floats are deployed

* comments

* docstrings and other codetools fixes

* rename SamplePoint to Spacetime and move them one dir up

* rm todo

* run pydocstyle on tests

* fix some names

* Update virtual_ship/instruments/ctd.py

Co-authored-by: Erik van Sebille <[email protected]>

* removed comment about ctd

* fix incorrect docstring

* renamed ship_st to ship_underwater_st

* remove sailship create fieldset commented out function

* ship underway st docstring improvement

* minor docstring change

* minor docstring change

---------

Co-authored-by: Erik van Sebille <[email protected]>
VeckoTheGecko pushed a commit that referenced this pull request Sep 25, 2024
* wip drifters

* drifters instrument. just missing test

* drifter test

* Remove old drifter file and fix errors

* codetools

* outputdf wherever i forgot to use it

* added todo list

* ctd instrument

* CTDtest

* use new ctd instrument in sailship

* fix typo

* codetools

* use new parcels api for particle class

* add ctd to init

* comments

* first try adcp

* adcp test

* use adcp instrument in sailship

* cleanup

* instrument ship s t

* use ship st in sailship

* cleanup

* refactor sailship ctd cast

* cleanup

* large cleanup

* add checks that all drifter and argo floats are deployed

* comments

* docstrings and other codetools fixes

* rename SamplePoint to Spacetime and move them one dir up

* rm todo

* run pydocstyle on tests

* fix some names

* Update virtual_ship/instruments/ctd.py

Co-authored-by: Erik van Sebille <[email protected]>

* removed comment about ctd

* fix incorrect docstring

* renamed ship_st to ship_underwater_st

* remove sailship create fieldset commented out function

* ship underway st docstring improvement

* minor docstring change

* minor docstring change

---------

Co-authored-by: Erik van Sebille <[email protected]>
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.

3 participants