Skip to content

docs: fix CometLogger log_model docstring example#21759

Open
gaurav0107 wants to merge 1 commit into
Lightning-AI:masterfrom
gaurav0107:fix/20583-comet-undocumented-and-incorrectly-descr
Open

docs: fix CometLogger log_model docstring example#21759
gaurav0107 wants to merge 1 commit into
Lightning-AI:masterfrom
gaurav0107:fix/20583-comet-undocumented-and-incorrectly-descr

Conversation

@gaurav0107

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the broken docstring example for log_model in the CometLogger
class (src/lightning/pytorch/loggers/comet.py). The current example is
syntactically invalid Python — it places a positional argument after a
keyword argument, and it omits the file_or_folder= keyword that the
Comet SDK requires.

Before:

logger.experiment.log_model(name="my-model", "<path to your model>")

After:

logger.experiment.log_model(name="my-model", file_or_folder="<path to your model>")

This matches the call signature documented at
https://www.comet.com/docs/v2/api-and-sdk/python-sdk/reference/Experiment/#experimentlog_model
and the worked example in issue #20583.

Fixes #20583

Before submitting
  • Was this discussed/agreed via a GitHub issue? — yes, Comet: Undocumented and incorrectly described function #20583, with a worked example by the reporter.
  • Did you read the contributor guideline?
  • Did you make sure your PR does only one thing? — yes, single-line docstring fix; no behavior changes.
  • N/A documentation update beyond the docstring itself.
  • N/A new tests — docstring-only change, nothing to test.
  • Did you verify new and existing tests pass locally with your changes? — ruff check and ruff format --check pass; AST parses cleanly.
  • N/A breaking changes.
  • N/A CHANGELOG — PR template explicitly excludes docstring fixes.

PR review

Anyone in the community is welcome to review the PR.

@gaurav0107
gaurav0107 marked this pull request as ready for review June 6, 2026 13:13
@gaurav0107
gaurav0107 requested a review from williamFalcon as a code owner June 6, 2026 13:13
@gaurav0107

Copy link
Copy Markdown
Contributor Author

Marked ready for review. This is a one-line docstring fix matching the corrected call signature in the issue body — no behavior change, ruff check and ruff format both pass locally. Happy to adjust if maintainers prefer a fuller rewrite of the surrounding log_model docs as a follow-up.

@codecov-commenter

codecov-commenter commented Jun 6, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79%. Comparing base (eea7443) to head (b20e94b).
⚠️ Report is 15 commits behind head on master.
✅ All tests successful. No failed tests found.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (eea7443) and HEAD (b20e94b). Click for more details.

HEAD has 894 uploads less than BASE
Flag BASE (eea7443) HEAD (b20e94b)
cpu 246 42
python 18 3
lightning_fabric 81 0
pytest 126 0
lightning 89 15
python3.11 36 6
python3.12 71 12
python3.10 17 3
python3.13 50 9
python3.12.7 54 9
pytorch2.3 9 3
pytest-full 120 42
pytorch2.8 15 6
pytorch_lightning 76 27
pytorch2.5.1 9 3
pytorch2.10 17 6
pytorch2.1 17 6
pytorch2.6 9 3
pytorch2.4.1 9 3
pytorch2.2.2 9 3
pytorch2.7 9 3
pytorch2.9 17 6
Additional details and impacted files
@@            Coverage Diff            @@
##           master   #21759     +/-   ##
=========================================
- Coverage      87%      79%     -8%     
=========================================
  Files         270      267      -3     
  Lines       24064    24005     -59     
=========================================
- Hits        20874    18871   -2003     
- Misses       3190     5134   +1944     

@gaurav0107
gaurav0107 force-pushed the fix/20583-comet-undocumented-and-incorrectly-descr branch from 798ade7 to efa33ef Compare July 2, 2026 15:08
@gaurav0107

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest master and force-pushed; the one-line docstring change itself is unchanged. The earlier docs-make (pytorch, doctest) failure was a transient CI issue rather than anything in this diff — this PR only edits a non-executed .. code-block:: python example, and doctest is green on master. It now passes on the rebased commit (efa33efa6). The only remaining red check is the external Read the Docs build, which isn't a required merge gate.

@gaurav0107
gaurav0107 force-pushed the fix/20583-comet-undocumented-and-incorrectly-descr branch from efa33ef to aedba1b Compare July 7, 2026 21:34
The example for `log_model` in the CometLogger class docstring was
syntactically invalid Python (positional argument after keyword) and
omitted the `file_or_folder=` keyword that the Comet SDK requires.

Update the rst code-block to use the documented call signature:

    logger.experiment.log_model(name="my-model", file_or_folder="<path to your model>")

Fixes Lightning-AI#20583
@gaurav0107
gaurav0107 force-pushed the fix/20583-comet-undocumented-and-incorrectly-descr branch from aedba1b to b20e94b Compare July 9, 2026 18:40
@gaurav0107

Copy link
Copy Markdown
Contributor Author

Rebased again onto the latest master (now at b20e94bfb) to re-trigger CI; the one-line docstring edit is unchanged.

The only substantive red was a single pl-cpu (ubuntu-22.04, pytorch, 3.12, 2.9) job failing at the Testing PyTorch step. This PR changes exactly one line inside a non-executed .. code-block:: python example in comet.py, so it cannot affect any pl-cpu test — every other pl-cpu matrix entry (macOS, Windows, and the other Linux versions) was green on the same run. It's unrelated master flakiness; the rebase should clear it. ruff check/ruff format --check pass locally and the corrected example now parses as valid Python (the original raised SyntaxError: positional argument follows keyword argument).

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.

Comet: Undocumented and incorrectly described function

2 participants