OSOceanAcoustics / echoshader

Interactive visualization of ocean sonar data
https://echoshader.readthedocs.io
MIT License
9 stars 8 forks source link

Final-version Refactoring #131

Closed ldr426 closed 1 year ago

ldr426 commented 1 year ago

This pull request aims to introduce the final version of the Refactoring for the Echoshader project. This PR aims to implements all functionalities with relative control widgets, dynamical box, and linkage to enhance the project's capabilities and user experience.

The PR includes four key files that provide essential functionalities:

Notes about this PR: https://docs.google.com/document/d/1QL89eLLcnLjhV8FRp4L_V1n52z0EbTwrrceGELBhVes/edit

Features built: https://github.com/OSOceanAcoustics/echoshader/issues/111 https://github.com/OSOceanAcoustics/echoshader/issues/112 https://github.com/OSOceanAcoustics/echoshader/issues/113 https://github.com/OSOceanAcoustics/echoshader/issues/117 https://github.com/OSOceanAcoustics/echoshader/issues/119 https://github.com/OSOceanAcoustics/echoshader/issues/115 https://github.com/OSOceanAcoustics/echoshader/issues/116 https://github.com/OSOceanAcoustics/echoshader/issues/86

small Issues solved: https://github.com/OSOceanAcoustics/echoshader/issues/107 https://github.com/OSOceanAcoustics/echoshader/issues/110 https://github.com/OSOceanAcoustics/echoshader/issues/99 https://github.com/OSOceanAcoustics/echoshader/issues/133 https://github.com/OSOceanAcoustics/echoshader/issues/132 https://github.com/OSOceanAcoustics/echoshader/issues/129 https://github.com/OSOceanAcoustics/echoshader/issues/127 https://github.com/OSOceanAcoustics/echoshader/issues/125 https://github.com/OSOceanAcoustics/echoshader/issues/96

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

valentina-s commented 1 year ago

@ldr426 Could you reorganize the repo to remove the new_version_3 folder so that it is ready for merging following the standard package structure.

  |-- echoshader
        |-- old_versions_folders (we can remove them once we settle )
        |-- ...
        |-- core.py
        |-- echogram.py
        |-- xyz.py (other scripts)
        |-- tests (a folder containing test scripts corresponding to each module)
              |-- test_core.py
              |-- test_echogram.py
              |-- test_xyz.py
  |-- notebooks
ldr426 commented 1 year ago

Hey @valentina-s, Thanks! Also need to update requirement.txt and add env creating command as I told you before: mamba create -n echoshader -c pyviz -c conda-forge hvplot geoviews pyvista ipykernel This related to https://github.com/OSOceanAcoustics/echoshader/issues/120

valentina-s commented 1 year ago

Also a few other notes:

valentina-s commented 1 year ago

Hey @valentina-s, Thanks! Also need to update requirement.txt and add env creating command as I told you before: mamba create -n echoshader -c pyviz -c conda-forge echopype hvplot geoviews pyvista ipykernel This related to #120

So if the assumption is that people have created the MVBS_Sv dataset, they would not need echopype any more.

valentina-s commented 1 year ago

@ldr426 I see that you have done the bidirectional logic. It is great that it worked out! Could you reorganize the folder structure to remove the new_version_3 folder, so that this PR is ready for merging. We should finalize this version before doing other things, those can be done as separate issues and PRs.

valentina-s commented 1 year ago

@ldr426 I managed the import and fiddled with several panel configurations of the widgets. Some of the interactive linking did not work for me, although one direction worked. I can test more with the new notebooks. For now just make the current one properly import the package. And change the PR from Draft to ready for review, so I can merge it.