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

docs: fix a few hyperlinks in the Python SDK reference. #8693

Merged
merged 2 commits into from
Jan 16, 2024
Merged

Conversation

ioga
Copy link
Contributor

@ioga ioga commented Jan 12, 2024

Description

I’ve been asked why did the docstring for the Experiment class https://docs.determined.ai/latest/reference/python-sdk.html#determined.experimental.client.Experiment
has determined.experimental.client.get_experiment() in it, but it’s not a hyperlink.

I went in and added :func:, :attr:, and :meth: annotations for it and a few classes nearby.
Also added docs autogen for a bunch of missing methods in determined.experimental.client, and missing User and ResourcePool class references.

Test Plan

Are things which look like methods and functions in the docs actually getting rendered as hyperlinks?

Commentary (optional)

If you find more broken or nonexisting links which are not in this PR, please make your own PR to fix them.

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

@ioga ioga requested a review from a team as a code owner January 12, 2024 23:38
@cla-bot cla-bot bot added the cla-signed label Jan 12, 2024
Copy link

netlify bot commented Jan 12, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 812683b
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/65a6ce09d13a3700089b6fe4

@determined-ci determined-ci requested a review from a team January 12, 2024 23:38
@determined-ci determined-ci added the documentation Improvements or additions to documentation label Jan 12, 2024
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (130ebb7) 51.50% compared to head (812683b) 46.32%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8693      +/-   ##
==========================================
- Coverage   51.50%   46.32%   -5.19%     
==========================================
  Files         887      726     -161     
  Lines      156212   144482   -11730     
  Branches     2088     2088              
==========================================
- Hits        80462    66932   -13530     
- Misses      74256    76057    +1801     
+ Partials     1494     1493       -1     
Flag Coverage Δ
harness 41.94% <100.00%> (-22.36%) ⬇️
web 53.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ined/common/experimental/checkpoint/_checkpoint.py 38.19% <ø> (-30.66%) ⬇️
...rness/determined/common/experimental/experiment.py 36.36% <ø> (-35.13%) ⬇️
harness/determined/common/experimental/model.py 34.96% <ø> (-39.88%) ⬇️
harness/determined/common/experimental/trial.py 37.74% <ø> (-17.65%) ⬇️
harness/determined/common/experimental/user.py 24.59% <ø> (-39.35%) ⬇️
harness/determined/experimental/client.py 55.26% <100.00%> (-3.68%) ⬇️

... and 241 files with indirect coverage changes

When false, filter for inactive users. Return all users otherwise.

Returns:
:class:`~determined.common.experimental.user.User`.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: actually returns a list of them

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, could you make please prefer determined.experimental.client.User to determined.common.experimental.user.User path? The former is the public path, even if Sphinx autodoc doesn't always pick the right one in the existing docs.

Tested on my system, link seems fine after changing to client.User.

Copy link
Contributor

@wes-turner wes-turner left a comment

Choose a reason for hiding this comment

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

One tiny requested change. Thanks for the improvements! I see anything else missing, I'll open my own PR.

@ioga ioga merged commit 06d7669 into main Jan 16, 2024
67 of 85 checks passed
@ioga ioga deleted the func-refs branch January 16, 2024 19:02
@dannysauer dannysauer modified the milestone: 0.27.1 Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants