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

Update API docs #1087

Merged
merged 12 commits into from
Nov 18, 2022
Merged

Update API docs #1087

merged 12 commits into from
Nov 18, 2022

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Nov 3, 2022

preview (see sidebar)

Description of proposed changes

Changes of all sizes to make the API docs reflect the latest state of things in the codebase.

Changes done outside of PR

Related issue(s)

Related to #1011

Testing

  • Manually click around RTD preview, spot-check for broken links

Checklist

  • Add a message in CHANGES.md summarizing the changes in this PR. Keep headers and formatting consistent with the rest of the file.

Post-merge tasks

  • Verify RTD redirects are synced.

@victorlin victorlin self-assigned this Nov 3, 2022
@victorlin victorlin force-pushed the victorlin/update-api-docs branch 2 times, most recently from ee1dd31 to 64ec6dc Compare November 3, 2022 23:43
@victorlin victorlin marked this pull request as ready for review November 3, 2022 23:43
@victorlin victorlin requested a review from a team November 3, 2022 23:43
@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Base: 63.32% // Head: 63.37% // Increases project coverage by +0.04% 🎉

Coverage data is based on head (bfdf551) compared to base (eba6b08).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head bfdf551 differs from pull request most recent head 610a337. Consider uploading reports for the commit 610a337 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1087      +/-   ##
==========================================
+ Coverage   63.32%   63.37%   +0.04%     
==========================================
  Files          57       57              
  Lines        6628     6637       +9     
  Branches     1632     1632              
==========================================
+ Hits         4197     4206       +9     
  Misses       2147     2147              
  Partials      284      284              
Impacted Files Coverage Δ
augur/__init__.py 66.66% <100.00%> (ø)
augur/align.py 89.28% <100.00%> (+0.04%) ⬆️
augur/export_v1.py 74.46% <100.00%> (ø)
augur/export_v2.py 69.08% <100.00%> (ø)
augur/filter.py 95.76% <100.00%> (+0.02%) ⬆️
augur/frequencies.py 51.66% <100.00%> (ø)
augur/index.py 88.15% <100.00%> (+0.32%) ⬆️
augur/io/__init__.py 100.00% <100.00%> (ø)
augur/mask.py 100.00% <100.00%> (ø)
augur/parse.py 89.41% <100.00%> (+0.12%) ⬆️
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

Looks good to me, though we should probably put in place a bunch of page-specific redirects for the /api/**/api/developer/** move?

# redirects.
#
# ¹ https://github.com/nextstrain/docs.nextstrain.org#configuring-redirects
---
Copy link
Member

Choose a reason for hiding this comment

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

We need to include the pre-existing redirects in RTD in this file, or they'll be deleted when sync is run!

You can see the pre-existing ones with

rtd projects nextstrain-augur redirects

and get them as JSON with:

rtd --json projects nextstrain-augur redirects

which can be lightly massaged into the redirects.yaml format.

Then double-check that the initial redirects.yaml matches state by doing a dry-run sync against it and checking that nothing's changed:

rtd projects nextstrain-augur redirects sync --dry-run -f redirects.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in force-push.

Output of `rtd projects nextstrain-augur redirects sync --dry-run -f docs/redirects.yaml` at bb92189
          Created           
                            
  Type   From URL   To URL  
 ────────────────────────── 
                            
          Deleted           
                            
  Type   From URL   To URL  
 ────────────────────────── 
                            
Created 0, deleted 0, kept 13.

No changes made in --dry-run mode.  Pass --wet-run for realsies.
Output of `rtd projects nextstrain-augur redirects sync --dry-run -f docs/redirects.yaml` at 50425d9
Creating: page /api/augur.export_v1.html → /api/developer/augur.export_v1.html
Creating: page /api/augur.utils.html → /api/developer/augur.utils.html
Creating: page /api/augur.align.html → /api/developer/augur.align.html
Creating: page /api/augur.util_support.node_data_reader.html → /api/developer/augur.util_support.node_data_reader.html
Creating: page /api/augur.lbi.html → /api/developer/augur.lbi.html
Creating: page /api/augur.distance.html → /api/developer/augur.distance.html
Creating: page /api/augur.export_v2.html → /api/developer/augur.export_v2.html
Creating: page /api/augur.frequency_estimators.html → /api/developer/augur.frequency_estimators.html
Creating: page /api/augur.index.html → /api/developer/augur.index.html
Creating: page /api/augur.ancestral.html → /api/developer/augur.ancestral.html
Creating: page /api/augur.util_support.node_data.html → /api/developer/augur.util_support.node_data.html
Creating: page /api/augur.titer_model.html → /api/developer/augur.titer_model.html
Creating: page /api/augur.util_support.html → /api/developer/augur.util_support.html
Creating: page /api/augur.util_support.node_data_file.html → /api/developer/augur.util_support.node_data_file.html
Creating: page /api/augur.frequencies.html → /api/developer/augur.frequencies.html
Creating: page /api/augur.tree.html → /api/developer/augur.tree.html
Creating: page /api/augur.clades.html → /api/developer/augur.clades.html
Creating: page /api/augur.sequence_traits.html → /api/developer/augur.sequence_traits.html
Creating: page /api/augur.titers.html → /api/developer/augur.titers.html
Creating: page /api/augur.parse.html → /api/developer/augur.parse.html
Creating: page /api/augur.traits.html → /api/developer/augur.traits.html
Creating: page /api/augur.mask.html → /api/developer/augur.mask.html
Creating: page /api/augur.util_support.color_parser.html → /api/developer/augur.util_support.color_parser.html
Creating: page /api/augur.import.html → /api/developer/augur.import.html
Creating: page /api/augur.io.html → /api/developer/augur.io.html
Creating: page /api/augur.util_support.color_parser_line.html → /api/developer/augur.util_support.color_parser_line.html
Creating: page /api/augur.refine.html → /api/developer/augur.refine.html
Creating: page /api/augur.filter.html → /api/developer/augur.filter.html
Creating: page /api/augur.html → /api/developer/augur.html
Creating: page /api/augur.validate.html → /api/developer/augur.validate.html
Creating: page /api/augur.export.html → /api/developer/augur.export.html
Creating: page /api/augur.reconstruct_sequences.html → /api/developer/augur.reconstruct_sequences.html
Creating: page /api/augur.util_support.date_disambiguator.html → /api/developer/augur.util_support.date_disambiguator.html
Creating: page /api/augur.measurements.html → /api/developer/augur.measurements.html
Creating: page /api/augur.version.html → /api/developer/augur.version.html
Creating: page /api/augur.translate.html → /api/developer/augur.translate.html
Creating: page /api/augur.validate_export.html → /api/developer/augur.validate_export.html
Creating: page /api/augur.filenames.html → /api/developer/augur.filenames.html
Creating: page /api/api.html → /api/developer/
                                                       Created                                                        
                                                                                                                      
  Type   From URL                                          To URL                                                     
 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 
  page   /api/augur.export_v1.html                         /api/developer/augur.export_v1.html                        
  page   /api/augur.utils.html                             /api/developer/augur.utils.html                            
  page   /api/augur.align.html                             /api/developer/augur.align.html                            
  page   /api/augur.util_support.node_data_reader.html     /api/developer/augur.util_support.node_data_reader.html    
  page   /api/augur.lbi.html                               /api/developer/augur.lbi.html                              
  page   /api/augur.distance.html                          /api/developer/augur.distance.html                         
  page   /api/augur.export_v2.html                         /api/developer/augur.export_v2.html                        
  page   /api/augur.frequency_estimators.html              /api/developer/augur.frequency_estimators.html             
  page   /api/augur.index.html                             /api/developer/augur.index.html                            
  page   /api/augur.ancestral.html                         /api/developer/augur.ancestral.html                        
  page   /api/augur.util_support.node_data.html            /api/developer/augur.util_support.node_data.html           
  page   /api/augur.titer_model.html                       /api/developer/augur.titer_model.html                      
  page   /api/augur.util_support.html                      /api/developer/augur.util_support.html                     
  page   /api/augur.util_support.node_data_file.html       /api/developer/augur.util_support.node_data_file.html      
  page   /api/augur.frequencies.html                       /api/developer/augur.frequencies.html                      
  page   /api/augur.tree.html                              /api/developer/augur.tree.html                             
  page   /api/augur.clades.html                            /api/developer/augur.clades.html                           
  page   /api/augur.sequence_traits.html                   /api/developer/augur.sequence_traits.html                  
  page   /api/augur.titers.html                            /api/developer/augur.titers.html                           
  page   /api/augur.parse.html                             /api/developer/augur.parse.html                            
  page   /api/augur.traits.html                            /api/developer/augur.traits.html                           
  page   /api/augur.mask.html                              /api/developer/augur.mask.html                             
  page   /api/augur.util_support.color_parser.html         /api/developer/augur.util_support.color_parser.html        
  page   /api/augur.import.html                            /api/developer/augur.import.html                           
  page   /api/augur.io.html                                /api/developer/augur.io.html                               
  page   /api/augur.util_support.color_parser_line.html    /api/developer/augur.util_support.color_parser_line.html   
  page   /api/augur.refine.html                            /api/developer/augur.refine.html                           
  page   /api/augur.filter.html                            /api/developer/augur.filter.html                           
  page   /api/augur.html                                   /api/developer/augur.html                                  
  page   /api/augur.validate.html                          /api/developer/augur.validate.html                         
  page   /api/augur.export.html                            /api/developer/augur.export.html                           
  page   /api/augur.reconstruct_sequences.html             /api/developer/augur.reconstruct_sequences.html            
  page   /api/augur.util_support.date_disambiguator.html   /api/developer/augur.util_support.date_disambiguator.html  
  page   /api/augur.measurements.html                      /api/developer/augur.measurements.html                     
  page   /api/augur.version.html                           /api/developer/augur.version.html                          
  page   /api/augur.translate.html                         /api/developer/augur.translate.html                        
  page   /api/augur.validate_export.html                   /api/developer/augur.validate_export.html                  
  page   /api/augur.filenames.html                         /api/developer/augur.filenames.html                        
  page   /api/api.html                                     /api/developer/                                            
                                                                                                                      
          Deleted           
                            
  Type   From URL   To URL  
 ────────────────────────── 
                            
Created 39, deleted 0, kept 13.

No changes made in --dry-run mode.  Pass --wet-run for realsies.

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks!

One FYI from the updated commit message:

+    The file is initialized with existing redirects, fetched with:
+
+        rtd --json projects nextstrain-augur redirects \
+          | yq '.[] | [{"type": "page", "from_url": .from_url, "to_url": .to_url}]' \
+          | sed 's/"//g'
+

You can avoid the sed by using the -y option to yq to emit YAML instead of JSON and use JQ syntax to avoid the repetition of field names:

| yq -y 'map({type, from_url, to_url})'

run: python3 -m pip install readthedocs-cli

- name: Sync redirects
run: rtd projects nextstrain-augur redirects sync -f doc/redirects.yaml --wet-run
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run: rtd projects nextstrain-augur redirects sync -f doc/redirects.yaml --wet-run
run: rtd projects nextstrain-augur redirects sync -f docs/redirects.yaml --wet-run

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in force-push.

- Capitalize Augur and Python
- Use reStructuredText to format `augur` as inline code
With refactors such as `measurements` and `io` being moved from one
category to another, there is less need for distinction between the two.
This change allows similar refactors to come with less docs changes.

Removing the headings also removes an intermediate TOC level.
The io page has been empty since refactoring to a package in 0e98435.
Docs have been empty since the refactor to a package in dc4f58b.
Future commits will modify contents and add a public section.
This explicitly defines Augur's Python interface in the terms of a
public API and a developer API.

For the public API, start with augur.io.read_metadata and
augur.io.read_sequences since these are commonly used by external
scripts.
Create a redirects file and set up automation to apply it to the RTD
project.

The file is initialized with existing redirects, fetched with:

    rtd --json projects nextstrain-augur redirects \
      | yq '.[] | [{"type": "page", "from_url": .from_url, "to_url": .to_url}]' \
      | sed 's/"//g'
Redirects account for the move of API docs to a new developer section,
minus the additional pages for augur.dates, augur.io.*, and
augur.measurements.* since those were never deployed before being moved.
The top-level exposure of * imports led to many indirect imports in the
internal codebase, which is not good practice since it is prone to
cyclic dependencies.
Broad inclusion of everything at the augur.io level can lead to indirect
imports from internal code, which is why the previous commit removed
those.

However, it is necessary to expose some functions at the top level so
API users do not have to use direct imports (e.g. from augur.io.metadata import read_metadata).
Instead of defining the members of the public API in both the init file
and the docs page, just define them in the init file and include all
imported members in the docs page.
@victorlin victorlin merged commit 4615e8d into master Nov 18, 2022
@victorlin victorlin deleted the victorlin/update-api-docs branch November 18, 2022 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants