Skip to content

chore: Use shiny's Makefile to allow for test commands to change over time #1161

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 12, 2024
Merged

chore: Use shiny's Makefile to allow for test commands to change over time #1161

merged 3 commits into from
Oct 12, 2024

Conversation

@schloerke
Copy link
Contributor

Related:

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

  • 💾 Refactor
  • ✨ Feature
  • 🐛 Bug Fix
  • 🔧 Optimization
  • 📝 Documentation
  • ✅ Test
  • 🐳 Other

Related issues

  • Re lated issue # NA
  • Closes # NA

Checklist

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

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

The default pytest command does cover some tests, but others are ran under default settings. By integrating the make commands, py-shiny can update the internal script as things change without need for updates to narwhals.

MarcoGorelli
MarcoGorelli reviewed Oct 11, 2024
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

nice, thanks @schloerke !

is there a way to keep the mypy command tested, to check we don't regress the type annotations?

@schloerke schloerke mentioned this pull request Oct 11, 2024
Merged
@schloerke
Copy link
Contributor Author

Good catch! I lost it in the shuffle yesterday.

The mypy check within py-shiny's repo is only there to test that it can run (because mypy sometimes doesn't like commented code)... not to find type errors. We do have type coverage using pyright that is tested within make check (along with other linting checks).

I've changed py-shiny's make narwhals-test-integration command to perform both type checking and test all narwhals related unit tests in posit-dev/py-shiny#1733. (Waiting for CI to auto merge in the next few mins)

schloerke
schloerke commented Oct 11, 2024
View reviewed changes
@schloerke
Copy link
Contributor Author

schloerke commented Oct 11, 2024 •
edited
Loading

Working type check: https://github.com/narwhals-dev/narwhals/actions/runs/11293892161/job/31413093004?pr=1161#step:10:48

-------- Checking types with pyright --------
pyright
0 errors, 0 warnings, 0 informations

schloerke
schloerke commented Oct 11, 2024
View reviewed changes
@AlessandroMiola AlessandroMiola mentioned this pull request Oct 11, 2024
Merged
10 tasks
MarcoGorelli
MarcoGorelli approved these changes Oct 12, 2024
View reviewed changes
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

wow amazing, thanks @schloerke !

@MarcoGorelli MarcoGorelli merged commit b056530 into narwhals-dev:main Oct 12, 2024
23 checks passed
@dangotbanned dangotbanned mentioned this pull request Aug 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarcoGorelli MarcoGorelli MarcoGorelli approved these changes

Assignees

No one assigned

Labels

internal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@schloerke @MarcoGorelli