tobac
tobac copied to clipboard
Developing the developer guide
This is a draft pull request to collaboratively work on the developer guide.
I have started to suggest some subsections and points to include based on the discussion in #265
Here we can look at how our progress looks like on read the docs: https://tobac--281.org.readthedocs.build/en/281/
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 60.71%. Comparing base (
2fe56c0
) to head (cf3c6bd
). Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #281 +/- ##
==========================================
+ Coverage 60.44% 60.71% +0.26%
==========================================
Files 23 23
Lines 3522 3541 +19
==========================================
+ Hits 2129 2150 +21
+ Misses 1393 1391 -2
Flag | Coverage Δ | |
---|---|---|
unittests | 60.71% <100.00%> (+0.26%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@JuliaKukulies : I now added the sphinx-based rendering docu to this PR. I used pandoc to convert markdown to rst, but I am not sure with all code parts have been converted in the correct way. Could you have a look?
Perfect @fsenf ! I added your file to index.rst
but for some reason the page does not show up. At a first glance the conversion looks good, but I will check in more detail.
I made some updates to the dev guide which will come with the next direct commit to the branch.
All looks good beside that these topics still need to be described in detail in doc/contributing.rst
- [ ] Github actions / workflow
- [ ] unit testing -> How to add a unit test
- [ ] jupyter notebook examples
It would be great to get support from @freemansw1 and @w-k-jones for describing the first two items. I could care for the last item.
Should make separate rst file for each topic to make the content easier to diggest?
Great, thanks for your updates @fsenf ! I think it is a good idea to have separate .rst files for these items as they are a bit more comprehensive. The structure of the dev guide is also just a first draft suggestion, so feel free to change it where necessary!
What should we do with the CONTRIBUTING.md? Do you want to keep it or should we just make sure that all the information is somewhere in the dev guide? I personally find it better to have all on one place and remove CONTRIBUTING.md
My suggestion would be:
- Keep CONTRIBUTING.md
- but reduce content to nice welcome words and a link list to the actual content in the dev guide
My suggestion would be:
- Keep CONTRIBUTING.md
- but reduce content to nice welcome words and a link list to the actual content in the dev guide
I like this suggestion- let's do that!
Linting results by Pylint:
Your code has been rated at 8.69/10 (previous run: 8.69/10, +0.00) The linting score is an indicator that reflects how well your code version follows Pylint’s coding standards and quality metrics with respect to the main branch. A decrease usually indicates your new code does not fully meet style guidelines or has potential errors.
I converted this from a draft pull request to a real one now and unless @fsenf does not have any oppositions, I think it is finally ready for review.
I have merged the latest changes from the RC branch and included sections on
- unit testing
- github actions
- jupyter notebooks
I have also made some minor modifications and clean-up to CONTRIBUTING.md
(see discussion above).
I think the developer guide does not need to be perfect to be merged as we continue working on it. However, a section where I would like particular attention from whoever reviews this PR is the section on migrating to xarray. I have included some informatioon on the decorators, but maybe it would actually be benefial for all of us developers to provide step-by-step instructions on how to work on this topic?
No objections at all!
@JuliaKukulies: Thank you for finalizing this very important contributions! I will try to make some time next week to contribute to the PR.
Thanks for your comments, @fsenf ! I think I addressed them all :)
@freemansw1 : Hi Sean, this PR is ready for a 2nd review. Do you think you can make it until the next dev meeting? Thanks, Fabian.
Who agreed to be the 2nd reviewer here? Was it you @freemansw1 ?
Thanks, @JuliaKukulies and @fsenf .
We have one minor merge conflict to fix, but I would also be happy for this to be merged directly into main
as it only touches documentation, per our policies around that kind of thing.
Not super sure why the zenodo json check fails, since the formatting and syntax seemed to be OK after i updated my affiliation?
All right, I fixed the json formatting issue, so I am merging directly into main
as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.
All right, I fixed the json formatting issue, so I am merging directly into
main
as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.
See the JSON formatting CI was making sure that you complied with all new marketing requirements :) (I still don't know what the issue was, but glad it resolved).
All right, I fixed the json formatting issue, so I am merging directly into
main
as we have discussed before. Thanks @fsenf and @freemansw1 and lets make sure to continue to work and update the developer guide in the future.See the JSON formatting CI was making sure that you complied with all new marketing requirements :) (I still don't know what the issue was, but glad it resolved).
Haha goood :)
Also not sure what the issue was, I just copied the json file from another branch and it worked