Skip to content

Conversation

@krassowski
Copy link
Contributor

@krassowski krassowski commented Apr 25, 2025

📝 Summary

Previously the parameters completions would show up empty or with useless NoneType() annotation. This PR adds a patch for marimo while awaiting a solution in jedi and/or pylsp. The related upstream issues are:

Before After
image image
Before After
image image
Before After
image image

🔍 Description of Changes

  • add a test ensuring that all marimo parameters have documentation extracted by (patched) jedi
  • add patch for jedi to extract the documentation
    • from docstrings for functions
    • for comments above attribute definitions for data classes
  • disable auto-import of marimo by jedi so that we use static analysis and can read comments for dataclasses to generate documentation
  • fix docstrings in marimo highlighted by tests

📋 Checklist

  • I have read the contributor guidelines.
  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • I have added tests for the changes made.
  • I have run the code and verified that it works as expected.

📜 Reviewers

@mscolnick

@vercel
Copy link

vercel bot commented Apr 25, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marimo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 1, 2025 5:30pm

@vercel
Copy link

vercel bot commented Apr 25, 2025

@krassowski is attempting to deploy a commit to the marimo Team on Vercel.

A member of the Team first needs to authorize it.

def patch_jedi_parameter_completion() -> None:
import re

from docstring_to_markdown import UnknownFormatError, convert
Copy link
Contributor

Choose a reason for hiding this comment

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

this may not be installed (like in wasm). it is still optional, but pulled in from pylsp. could we put a try catch or use if DependencyManager.docstring_to_markdown.has():

@mscolnick mscolnick changed the title Improve documentation of parameters as show in the completion box improvement: documentation of parameters as show in the completion box May 1, 2025
@mscolnick mscolnick marked this pull request as ready for review May 1, 2025 15:58
@mscolnick mscolnick merged commit af222b1 into marimo-team:main May 1, 2025
33 checks passed
@krassowski krassowski deleted the parameters branch May 2, 2025 07:56
mscolnick added a commit that referenced this pull request May 5, 2025
…4786)

## 📝 Summary

Small follow up to #4673.

## 🔍 Description of Changes

Polars often uses a simplified rst format for parameters where it does
not include types.

This requires `docstring-to-markdown` v0.17 with:
- python-lsp/docstring-to-markdown#44

For example, before there was only one parameter with description for
`polars.io.parquet.functions.read_parquet`, and now all 22 are
displayed.

Parameters from functions with overloads such as
`polars.io.database.functions.read_database` remain without good
suggestions.

## 📋 Checklist

- [x] I have read the [contributor
guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md).
- [ ] For large changes, or changes that affect the public API: this
change was discussed or approved through an issue, on
[Discord](https://marimo.io/discord?ref=pr), or the community
[discussions](https://github.com/marimo-team/marimo/discussions) (Please
provide a link if applicable).
- [x] I have added tests for the changes made.
- [x] I have run the code and verified that it works as expected.

## 📜 Reviewers

@mscolnick

---------

Co-authored-by: Myles Scolnick <myles@marimo.io>
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