CDCgov / multisignal-epi-inference

Python package for statistical inference and forecast of epi models using multiple signals
https://cdcgov.github.io/multisignal-epi-inference/
9 stars 1 forks source link

Adding Repository Structure And Resources Sections To The README #197

Closed AFg6K7h4fhy2 closed 1 week ago

AFg6K7h4fhy2 commented 2 weeks ago

This is a small pull request that adds two simple items to the README file: the structure of the MSR repository via tree and a Resources section.

A Repository Structure section enables users to get an impression of the MSR repository very quickly. While MSR's structure will change, requiring the Repository Structure section of the README to be manually revised, revisions need not be done frequently, since the design of MSR currently is such that further updates are unlikely to deviate significantly enough for the existing but outdated tree output to cease to be useful. The only request I have here for reviewers is to provide some thoughts for what to ignore when running tree.

Resources are helpful for newcomers and for those who've forgotten where to find particular items relevant to the project. I have added four resources. More resources can be added by others, including the reviewers.

As for the placement of the sections in the README, I thought that after the General Disclaimer worked well, since Public Domain Standard Notice becomes less MSR and or CDC specific.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.76%. Comparing base (81c49f1) to head (c62b7ab). Report is 4 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #197 +/- ## ========================================== + Coverage 94.64% 94.76% +0.12% ========================================== Files 39 40 +1 Lines 840 898 +58 ========================================== + Hits 795 851 +56 - Misses 45 47 +2 ``` | [Flag](https://app.codecov.io/gh/CDCgov/multisignal-epi-inference/pull/197/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/CDCgov/multisignal-epi-inference/pull/197/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov) | `94.76% <ø> (+0.12%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AFg6K7h4fhy2 commented 1 week ago

@damonbayer I agree with you that a one sentence description can help. I still believe that having the repository structure displayed in the README would be a net good (perhaps if a Last Updated: YYYY-MM-DD) if provided somewhere. I am fine with this somewhere being the developer documentation while leaving a note on how to display the structure for oneself, as you suggested.

AFg6K7h4fhy2 commented 1 week ago

Is this

I think the model equation sheet could be moved to the documentation along with a brief introduction to the renewal model.

expected to be covered in #191 ?

@sbidari

sbidari commented 1 week ago

Is this

I think the model equation sheet could be moved to the documentation along with a brief introduction to the renewal model.

expected to be covered in #191 ?

@sbidari

Yes, just requested review for some proposed changes in documentation.

damonbayer commented 1 week ago

@damonbayer I agree with you that a one sentence description can help. I still believe that having the repository structure displayed in the README would be a net good (perhaps if a Last Updated: YYYY-MM-DD) if provided somewhere. I am fine with this somewhere being the developer documentation while leaving a note on how to display the structure for oneself, as you suggested.

I think it would be more helpful to give some general description (perhaps spread among readme.md, pipeline/readme.md, model/readme.md, and the dev documentation) of the structure. For example in the present readme, we already say

The library and pipeline are located in the model/ and pipeline/ directories of the GitHub repository, respectively.

That is enough info to get someone headed down the right path. Then in model/readme.md, we could say something like "the package is split up among x sub-modules". But this is already evident from the "reference" section of the website, so I'm not sure it's that helpful.

Printing the tree doesn't provide a lot of insight, and it is likely to often be out of date/annoying to maintain.