-
Notifications
You must be signed in to change notification settings - Fork 17
INTPYTHON-653 & INTPYTHON-620 Make pandas optional and add free-threading support on Windows #324
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
Conversation
try: | ||
from pandas.tests.extension import base | ||
except ImportError: | ||
pytest.skip("skipping pandas tests", allow_module_level=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some thing like "To include, create environment like so: uv sync --dev --extra test --extra test-polars --extra test-pandas
"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's already part of just test
, that seems redundant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. As long as we are steering contributors to the justfile
, that they've opened it, we're good.
except ImportError: | ||
self.skipTest("Test requires pandas") | ||
arrow_schema = {k.__name__: v(True) for k, v in _TYPE_NORMALIZER_FACTORY.items()} | ||
schema = {k: v.to_pandas_dtype() for k, v in arrow_schema.items()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use pandas to go from arrow to numpy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for this test, to get the right data types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Addresses #286.