Closed emiliom closed 3 years ago
@leewujung I compared the docs files between master
and class-redesign
, and unfortunately there is a mix of updates in both branches :sad:. Fortunately, the conflict only involves two files, resources.rst
and usage.rst
.
usage.rst
: Most of the updates involve the file export method renaming from, eg, .raw2nc
to .to_netcdf
. These are in class-redesign
. Others are definitely more up-to-date in master
, and others I can't tell which branch should be used. The latter involve the description of EA640 as being Kongsberg or Simradresources.rst
: Looks to me like master
reflects the actual updates.I see that Simrad is a company that has been absorbed by Kongsberg. I'm guessing the EA640 came from Kongsberg and EK60 & EK80 from Simrad? But whether all 3 should be labelled as "Kongsberg Simrad" or just Simrad is confusing. What should we use?
Ok, what I'll do is come up with the best I can in terms of resolved conflicts with document files (from master
vs class-redesign
) in a new docs
branch. Once I push that branch to the repo, you can check it out and issue PR's on it to correct anything I didn't get right.
usage.rst: Most of the updates involve the file export method renaming from, eg, .raw2nc to .to_netcdf. These are in class-redesign.
Yes.
resources.rst: Looks to me like master reflects the actual updates.
Yes I think so too.
The latter involve the description of EA640 as being Kongsberg or Simrad
I believe EA640 is Kongsberg EA640, EK60 and EK80 are Kongsberg Simrad, as you have inferred.
In resources.rst, the link for the "Sonar4 and Sonar5-Pro" (http://folk.uio.no/hbalk/sonar4_5/index.htm) is broken; it leads to a page in Norwegian saying the url doesn't exist. I Googled Sonar5-Pro, and the most promising link I found was to a Kongsberg page that also uses the broken link.
Hm, last time I checked it it was still active. Not sure what the latest updated link is...
I've reconciled docs updates from master
and class-redesign
in a new branch, docs
. I'll use this branch for additional updates for the time being, at least until class-redesign
is merged into master
.
Only resources.rst
and usage.rst
were changed. For resources.rst
, it's basically all from master
, plus a couple of tiny cleanups. usage.rst
was the tricky one. @leewujung please take a look and correct anything that I didn't get right.
https://github.com/OSOceanAcoustics/echopype/blob/docs/docs/source/usage.rst
In addition to reconciling the two branches, I made a few small changes for clarification, consistency, etc. Now I'm ready to start working on #223!
Ref https://github.com/OSOceanAcoustics/echopype/commit/159356f45babc9bab4fb9307320994e12a318221, the commit that created the docs
branch
Add a .. warning::
block about the new default location for converted files (along with all the other things from raw2nc
/raw2zarr
to to_netcdf
/to_zarr
). ref #251
NOTE: currently there's no pre-built deployment of the readthedocs updates; you can only browse the markdowns in the docs
branch or pull the branch and build sphinx locally. I'm now building the sphinx docs locally, and will work on pushing the built site somewhere online so everyone can see them easily.
Hold on! I actually have that set up on a separate test readthedocs site. I am in a meeting atm but I can ping you afterward.
Pasting @leewujung's info regarding the test readthedocs site:
This is the doc test site i created separately https://readthedocs.org/projects/doc-test-echopype/
i added you as maintainer. you could change the repo address under admin options to be your fork. then you can test it and we can view it
I've decided that it's best to re-organize the docs a fair bit. The usage page was getting too big. So:
usage.rst
likely into 3 parts (installation + convert + processing)I've already started this work (esp. 1), and will hopefully push these changes to the readthedocs test site before Monday.
Woo-hoo, I'm now able to push to the test docs site via my fork! Here are my changes so far: https://doc-test-echopype.readthedocs.io/en/latest/
Please ignore the "File access options" in usage.rst
for now; it's a mess of pasted text and a scratchpad of ideas. That's the section I'm working on.
I like the reorganization! 😀
Thanks for chiming in! I took that as license to push ahead and split usage.rst. See the result :grinning: https://doc-test-echopype.readthedocs.io/en/latest/ Check it out.
I'm done for the day.
S3 documentation for both read and write are now in the docs branch and test readthedocs. Both in draft form until everything is tested and remaining details are ironed out. ref: #256
Btw, I noticed that the copyright notice is 2018-
and we are now in 2021
. Also, the copyright person/org is currently Wu-Jung, should this change to OSOceanAcoustics? :smile:
Add instructions on how to set up the environment for local testing (ie, what's implemented in GitHub actions). See steps in #260, plus my notes
In a section about contributing to development, mention that the GH Actions CI (ci.yaml
) can be skipped by including the string "[skip ci]" in the commit message. See ci.yaml#L22. If the push or PR has multiple commits, does the string have to be in the most recent (header?) commit?
I think we could improve and update https://echopype.readthedocs.io/en/latest/why.html a bit. Nothing major. What comes to mind is adding something about Zarr and cloud support, and some proof reading.
Remove text about use of Git LFS. See #271
Another section that requires update (now that we've come to near the surface of overhauling convert!) is the section to specify metadata type of attributes required by SONAR-netCDF4 when converting files. A new method set_param
is introduced in the new Convert
object to handle this.
This is a component I marked with TODOs for before v0.5.0 and I guess we're there now. Just a heads-up at the moment but I'll ping you here @emiliom once I review set_param
and update the docstring.
Sounds good, thanks.
And notes to self:
open_raw
instead of Convert
(while still mentioning Convert
)open_raw
quick notes:
From Don: PR #302 should allow for easy test run.
python .ci_helpers/docker/setup-services.py --deploy
python .ci_helpers/run-test.py --local --pytest-args="-vv"
This is closed by #337.
Creating new issue to serve as parent to related docs issues (#223, #234) and add new elements.