Skip to content

Fix a typo and two copy-pasted docstrings in the telemetry module - #80

Open
Avicennasis wants to merge 1 commit into
PriorLabs:mainfrom
Avicennasis:docs/fix-typos
Open

Avicennasis wants to merge 1 commit into
PriorLabs:mainfrom
Avicennasis:docs/fix-typos

Conversation

@Avicennasis

Copy link
Copy Markdown

Summary

Three comment/docstring fixes, no functional changes.

  • src/tabpfn_common_utils/telemetry/core/service.py: # Supress warnings -> # Suppress warnings (the block comment two lines above already spells it "Suppress").
  • src/tabpfn_common_utils/telemetry/interactive/prompts/identity.py (_should_prompt): "Check if the user should be prompted to subscribe to the newsletter." -> "...to share anonymous usage analytics." The line is copied from newsletter.py; this module is the identity/analytics prompt (kind="identity", title "Share anonymous usage analytics?").
  • src/tabpfn_common_utils/telemetry/interactive/prompts/identity.py (_on_done): "Done callback for newsletter prompt." -> "Done callback for identity prompt."

The matching lines in prompts/newsletter.py are correct there and were left untouched. Comments and docstrings only - ruff/pyright behaviour is unaffected.

- telemetry/core/service.py: 'Supress' -> 'Suppress'
- telemetry/interactive/prompts/identity.py: two docstrings copied from
  newsletter.py now describe the identity prompt

Comments and docstrings only; no functional changes.

Signed-off-by: Léon Avic Simmons <Avicennasis@gmail.com>
@Avicennasis
Avicennasis requested a review from a team as a code owner September 7, 2026 09:20
@Avicennasis
Avicennasis requested review from anuragg1209 and removed request for a team September 7, 2026 09:20

@anuragg1209 anuragg1209 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for the PR @Avicennasis!

@anuragg1209 anuragg1209 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you please fix the failing Ruff test case? Thanks!

@Avicennasis

Copy link
Copy Markdown
Author

Hi @anuragg1209 — happy to help, but I think this one isn't coming from my change, and I'd rather flag that than quietly expand the PR.

This PR changes 3 lines of comment/docstring text in 2 files. I checked out clean main (3e4c992, none of my changes) and ran the same ruff the CI action installs:

ruff 0.16.8, `ruff check .` on clean main  ->  Found 140 errors
ruff 0.16.8, `ruff check .` with PR #80    ->  Found 140 errors

Delta from this PR: 0. None of the reported errors are on lines the PR touches — the CI log points at src/tabpfn_common_utils/utils.py and seven files under tests/, none of which this PR opens.

The cause looks like an unpinned lint dependency rather than anything in the diff. pyproject.toml has:

"ruff>=0.11.6",

and .github/workflows/pull_requests.yml uses astral-sh/ruff-action, which installs the latest ruff. Ruff 0.16.x turned on rules the codebase predates — FA100, I001, UP006/UP007/UP035, SIM117, BLE001. Locally, ruff 0.15.20 reports All checks passed! on the very same commit, so the last green run on main simply ran before 0.16.8 was released. I'd expect main to go red on its next scheduled run regardless of this PR.

Two ways I can help, whichever you prefer:

  1. Pin the linter — a one-line change (e.g. "ruff>=0.11.6,<0.16") restores green immediately and keeps the upgrade a deliberate decision. Happy to open that as a separate PR.
  2. Do the cleanup — also happy to open a separate PR fixing the 140 findings (33 are auto-fixable; the FA100/UP ones interact with target-version = "py39", so they want a considered approach rather than --fix --unsafe-fixes).

I'd suggest keeping either of those out of this PR so the typo fix stays reviewable — but if you'd rather I just fold the lint fixes in here, say the word and I'll do that instead.

This branch has not been deployed

No deployments
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.

2 participants