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

Add docs #107

Merged
merged 49 commits into from
Oct 6, 2023
Merged

Add docs #107

merged 49 commits into from
Oct 6, 2023

Conversation

ejm714
Copy link
Collaborator

@ejm714 ejm714 commented Oct 3, 2023

First pass at some docs

Outstanding:

  • add model performance page -> Write model performance page #108
  • add changelog.md and code to copy it into a docs page
  • add github actions set up on render
  • some kind of visual
  • render csv rows as tables in the about the model section

Closes #66

@ejm714 ejm714 requested a review from klwetstone October 3, 2023 22:22
@render
Copy link

render bot commented Oct 4, 2023

Copy link
Collaborator

@klwetstone klwetstone left a comment

Choose a reason for hiding this comment

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

@ejm714 these are excellent!! Extremely easy to follow, well written, and covers all the key pieces of information. I love it!

Most of my comments are copy editing, and are up to you whether to incorporate.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/docs/about.md Outdated Show resolved Hide resolved
docs/docs/index.md Outdated Show resolved Hide resolved
docs/docs/installation.md Outdated Show resolved Hide resolved
docs/docs/index.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/mkdocs.yml Outdated Show resolved Hide resolved
docs/mkdocs.yml Outdated Show resolved Hide resolved
ejm714 and others added 14 commits October 4, 2023 15:51
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
Co-authored-by: Katie Wetstone <46792169+klwetstone@users.noreply.github.com>
@ejm714
Copy link
Collaborator Author

ejm714 commented Oct 5, 2023

Thanks for the comments @klwetstone! Could you take the two outstanding items? I think it'd be nice to add some kind of visual. I was thinking on the home page but that likely puts the quickstart too far below the fold so maybe an image is better on the about page. Also you can make the dataframes tables in html and that will render fine. If you just use the markdown table format, they get assigned 100% width (and we should use auto width sizing so this works across screen types) so you may want to fiddle with the css in style.css. Previews are set up on render (https://cyfi-pr-107.onrender.com/) but you can also serve locally for developing.

@klwetstone
Copy link
Collaborator

@ejm714 Done!

  • I didn't use the markdown table format so I could wrap some tables in a new .table-container-class, and add a horizontal scroll bar
  • I rounded the feature values in the example features row so it's a little easier to navigate
  • I added an image to the homepage, and put it next to the intro text so it doesn't push the quickstart down too far. I think this works pretty well, but could be improved if we added code to hide the image when the screen is too narrow. If that's appealing I can fiddle with it a bit more.

Two questions:

  • Do we use docs/docs/quickstart.md anywhere? It doesn't look like we do, in which case we can get rid of that page.
  • Should we add the files in docs/site that are generated by make docs to our gitignore?

@ejm714
Copy link
Collaborator Author

ejm714 commented Oct 6, 2023

Thanks @klwetstone -- I made the following adjustments

  • resized the image to be more rectangular, used a jpeg for much faster loading, and use the built in bootstrap functionality so it gets wrapped below the text when the screen gets narrow
  • updated the tables to use bootstrap classes that do nice things with sizing and scrollbars (i found the stripes confusing so set it to all one color, could be improved b/c it's a lot of gray but it's okay for now)
  • removed quickstart.md and added docs/site to .gitignore ✔️

@klwetstone
Copy link
Collaborator

@ejm714 This all looks great! Especially the homepage -- much better.

I just made a few tiny tweaks:

  • I wrapped a few more tables in a table-responsive class div so that they would get a scrollbar, rather than overflowing width-with and adding whitespace to the right of the content.
  • There was one table row missing a
  • I changed "Tick Tick BLoom" to "Tick Tick Bloom" in a few places
  • I ran the homepage image through ImageOptim to make it slightly smaller. It still takes a minute to load

I also checked all the external links and all of them worked.

I noticed that links to other pages within the documentation are a mix of relative paths (eg. the link to the quickstart page from installation.md) and urls (eg. the link to the installation page from the index.md). Currently only the former works (as expected), I wanted to flag in case we have a preference and want to make them uniform.

I think this all looks EXCELLENT! Well done!

@ejm714
Copy link
Collaborator Author

ejm714 commented Oct 6, 2023

I think the links are set up correctly but I made a note in #34 to double check once the docs are deployed.

@ejm714 ejm714 merged commit 03eac66 into main Oct 6, 2023
3 checks passed
@ejm714 ejm714 deleted the docs branch October 6, 2023 18:14
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.

Write package docs
2 participants