-
Notifications
You must be signed in to change notification settings - Fork 59
Fix: index not shown when polars is not installed #416
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
|
Thank you for making this pull request. Did you know? You can try it on Binder: Also, the version of ITables developed in this PR can be installed with (this requires |
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.
Pull Request Overview
This PR fixes an issue where the DataFrame index was not shown when Polars was not installed by adjusting the _evaluate_show_index logic, adds a test to verify index display, bumps the version to 2.4.3, and updates the changelog.
- Adjust
_evaluate_show_indexto compareplagainstpdso absence of Polars still shows index. - Add
test_get_itable_arguments_with_indexto ensure indices are included indata_json. - Bump version and document the fix in
changelog.md.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_javascript.py | Added a new test to verify index inclusion when Polars is not installed |
| src/itables/version.py | Bumped version from 2.4.2 to 2.4.3 |
| src/itables/javascript.py | Updated _evaluate_show_index condition to use pl is not pd |
| docs/changelog.md | Added 2.4.3 changelog entry for the Polars index fix |
Comments suppressed due to low confidence (2)
tests/test_javascript.py:46
- [nitpick] The docstring here doesn't describe the new test’s intent. Consider updating it to explain that this test verifies index display when Polars is not installed.
As much as possible we want few arguments for the ITable class
docs/changelog.md:8
- [nitpick] Consider rephrasing for clarity, e.g., 'Fixed issue where index was not shown when Polars was not installed.'
- We have fixed an issue with an index not shown when `polars` was not installed ([#415](https://github.com/mwouts/itables/issues/415))
| if showIndex != "auto": | ||
| return showIndex | ||
| if df is None: | ||
| return False |
Copilot
AI
Jul 1, 2025
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.
[nitpick] Add a comment explaining why we're comparing 'pl' to 'pd' here, so future readers understand the aliasing logic when Polars is not available.
| return False | |
| return False | |
| # If Polars is not available, `pl` is aliased to `pd`. This check ensures | |
| # that we correctly handle Polars DataFrames when Polars is available. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #416 +/- ##
==========================================
+ Coverage 87.79% 87.82% +0.03%
==========================================
Files 50 50
Lines 2007 2012 +5
==========================================
+ Hits 1762 1767 +5
Misses 245 245 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #415