-
Notifications
You must be signed in to change notification settings - Fork 10
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
Remodelling the documentation #462
Conversation
Adding ``csv`` instead of csv, Python instead of python, and Jupyter instead of jupyter
…ambiguous docstrings #463
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I built this here to check: https://mapreader.readthedocs.io/en/kallewesterling-issue460/
I realise now my comment re. numbering might not be a problem since the numbers don't show in the docs. Might be useful still to indicate the workflows for classification vs text.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment is potentially to remove the numbering from this or somehow make it clear that you can do:
Classficiation:
- download->load->annotate->classify
- download->load->classify (e.g. with pretrained model)
- the above + post processing
- the above - download
Text spotting:
- download->load->spot-text
- the above - download
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, so you mean that:
- the download step is sort-of optional (as long as you have downloaded the maps already)
- the post-processing step is an optional tag-on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep and if you are doing text spotting you don't have to do classify too
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #462 +/- ##
=======================================
Coverage 62.77% 62.77%
=======================================
Files 41 41
Lines 6807 6807
=======================================
Hits 4273 4273
Misses 2534 2534
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I had a go at fixing merge conflicts here, mostly around the |
Sounds good -- let me look at your comment before we do though! |
This looks great to me! Prompts me to ask @kallewesterling if we can set up some kind of reminder system to add to the 'project cv' and other elements that require updates. |
I think we should just have that as a standing point in our sprint meeting to keep revisiting. I'll add it to my idea board for Thursday meetings |
Summary
The documentation had unclear ordering and files that were named in ways that didn't optimise the SEO of our documentation. That should be fixed with this (see left image below, compared to the former structure on the right).
Fixes #460
Fixes #463
Describe your changes
Checklist before assigning a reviewer (update as needed)
Reviewer checklist
Please add anything you want reviewers to specifically focus/comment on.