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

ci: add tubular in ci checks #1211

Merged
merged 9 commits into from
Oct 23, 2024
Merged

ci: add tubular in ci checks #1211

merged 9 commits into from
Oct 23, 2024

Conversation

FBruzzesi
Copy link
Member

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.

@FBruzzesi
Copy link
Member Author

Can't replicate the issue locally even with fresh new env. (link to failing action)

Error:

INTERNALERROR> pytest.PytestConfigWarning: Unknown config option: env

@DeaMariaLeon
Copy link
Member

DeaMariaLeon commented Oct 18, 2024

@FBruzzesi Would that have something to do with this line in Narwhals' pyproject.toml? :
env = [ "MODIN_ENGINE=python", ]

If so, should that environment variable be added? (to the tubular test section on .github/workflows/downstream_tests.yml that is being added).. Asking for a friend 🥸

@FBruzzesi
Copy link
Member Author

@FBruzzesi Would that have something to do with this line in Narwhals' pyproject.toml? : env = [ "MODIN_ENGINE=python", ]

If so, should that environment variable be added? (to the tubular test section on .github/workflows/downstream_tests.yml that is being added).. Asking for a friend 🥸

I wonder why none of the other downstream test ends up with similar issue if that was the case! I will try to add the env variable (and pytest-env)

@DeaMariaLeon
Copy link
Member

It went further!..

@FBruzzesi
Copy link
Member Author

FBruzzesi commented Oct 18, 2024

It went further!..

Yep yep, that was a good catch Dea, thanks for spotting it! Next issue ahead 😁🙈

Edit: The error was raised due to the filterwarnings settings in narwhals itself. By explicitly declaring which config file to use (pytest tests --config-file=pyproject.toml), locally I am not able to reproduce the issue.

I wonder if we should do the same with the other downstream libraries

@FBruzzesi FBruzzesi marked this pull request as ready for review October 19, 2024 10:44
@FBruzzesi
Copy link
Member Author

Regarding marimo typecheck failing, seems we are missing some libraries?

@DeaMariaLeon
Copy link
Member

I'm not sure if requesting me as a reviewer was an accident or a github bug 😆, but LGTM.

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.

thanks @FBruzzesi and @DeaMariaLeon !

@MarcoGorelli MarcoGorelli merged commit 23eec12 into main Oct 23, 2024
29 checks passed
@FBruzzesi FBruzzesi deleted the ci/tubular branch October 23, 2024 15:10
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.

ci: add downstream tests for tubular
3 participants