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

test: add docstrings mypy check #1448

Closed

Conversation

LiamConnors
Copy link
Member

@LiamConnors LiamConnors commented Nov 26, 2024

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes

If you have comments or can explain your changes, please do so below

@LiamConnors LiamConnors changed the title Add docstrings mypy check test: add docstrings mypy check Nov 26, 2024
@LiamConnors
Copy link
Member Author

The test takes a bit too long to run (15-20minutes). Will see if there's another way to go about it.

It did find issues though

Checking narwhals/stable/v1/__init__.py all
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py col
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py nth
<string>:17: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py len
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py lit
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py min
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py max
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py mean
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py median
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py sum
<string>:15: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py sum_horizontal
<string>:17: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py all_horizontal
<string>:20: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

Checking narwhals/stable/v1/__init__.py any_horizontal
<string>:20: error: Name "IntoFrameT" is not defined  [name-defined]
Found 1 error in 1 file (checked 1 source file)

@EdAbati
Copy link
Contributor

EdAbati commented Nov 27, 2024

Thank @LiamConnors for looking into this! πŸ‘

I think the slowness comes from running mypy sequentially for each docstring.

For each module we could try to add all its docstrings into a separate temp file, and pass all these files at once to mypy. But we would need to show which files and docstring generates an error somehow. It'd be nice to have something like:

narwhals/stable/v1/__init__.py:any_horizontal:<line_no> error: Name "IntoFrameT" is not defined  [name-defined]

Is there something we can learn from nbqa @MarcoGorelli? If I am not mistaken, it's kind of similar, instead of multiple cells here we have multiple docstrings

@MarcoGorelli
Copy link
Member

thanks a tonne for doing this!

It did find issues though

nice, I did suspect we'd find something which --doctest-modules wasn't picking up

Maybe even just running python on each example would be a good start, and be fast enough?

@LiamConnors
Copy link
Member Author

Thanks @EdAbati and @MarcoGorelli for the feedback! Will take another look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants