-
Notifications
You must be signed in to change notification settings - Fork 0
BUG: Fix ValueError in Series.info(show_counts=False) #7
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
base: trunk-pr62699
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1097,6 +1097,7 @@ I/O | |
| - Bug in :meth:`set_option` where setting the pandas option ``display.html.use_mathjax`` to ``False`` has no effect (:issue:`59884`) | ||
| - Bug in :meth:`to_csv` where ``quotechar``` is not escaped when ``escapechar`` is not None (:issue:`61407`) | ||
| - Bug in :meth:`to_excel` where :class:`MultiIndex` columns would be merged to a single row when ``merge_cells=False`` is passed (:issue:`60274`) | ||
| - Bug in :meth:`~pandas.Series.info` where calling with ``show_counts=False`` raised a :exc:`ValueError` (:issue:`62590`) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good addition to the whatsnew document. This correctly documents the bug fix for
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good job adding a clear and concise entry to the "whatsnew" documentation. The entry correctly describes the bug that was fixed (ValueError in Series.info when show_counts=False) and properly references the issue number.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whatsnew entry is properly formatted and placed in the correct I/O section. The description clearly explains the bug and references the correct issue number.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whatsnew entry is properly formatted and placed in the correct I/O section. The description clearly explains the bug and references the appropriate issue number.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Appropriate documentation of the bug fix. The entry correctly references the issue number and describes the problem clearly. |
||
|
|
||
| Period | ||
| ^^^^^^ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -1096,7 +1096,7 @@ def headers(self) -> Sequence[str]: | |||||||
|
|
||||||||
| def _gen_rows_without_counts(self) -> Iterator[Sequence[str]]: | ||||||||
| """Iterator with string representation of body data without counts.""" | ||||||||
| yield from self._gen_dtypes() | ||||||||
| yield from ([dtype] for dtype in self._gen_dtypes()) | ||||||||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous implementation returns an iterator of strings directly from
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix! This change properly ensures that
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype string in a list to create a sequence. This ensures consistency with the expected return type of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to create a sequence. This ensures consistency with the expected return type of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype string in a list to create a sequence. This ensures consistency with the expected return type of
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the issue where
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the issue where The solution wraps each dtype string in a list to create the expected sequence format, which matches the pattern used in the DataFrame equivalent method
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the issue where The solution wraps each dtype string in a list to create the expected sequence format, which matches the pattern used in the DataFrame equivalent method
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix! The issue was that The fix correctly wraps each dtype string in a list
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix! The issue was that
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix! The issue was that
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The method signature indicates it should return
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to match the expected sequence format. This is consistent with the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to create proper sequences for the table builder. This change aligns with the expected format where each row should be a sequence of strings, not individual strings.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring However, consider adding a test case to prevent regression of this bug. The fix looks correct but would benefit from test coverage to ensure
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the issue where
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the issue by wrapping each dtype in a list to create proper sequence rows. This ensures consistency with the expected format for table building where each row should be a sequence of strings.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix! The change correctly wraps each dtype in a list to match the expected sequence format for table rows. This addresses the root cause of the ValueError when
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The method signature expects
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The method signature indicates it should return
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring each dtype is wrapped in a sequence. The
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to create the expected sequence format. However, consider using a more explicit approach for better readability and consistency with the
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring each dtype is wrapped in a sequence. This matches the expected format for table rows and is consistent with the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring each dtype is yielded as a sequence rather than a string. The
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by ensuring each dtype is wrapped in a sequence. The
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The method signature indicates it should return
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the type mismatch issue. The method signature expects
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to create sequences. This ensures consistency with the expected return type
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fix correctly addresses the ValueError by wrapping each dtype in a list to create the expected sequence format. This ensures consistency with the table builder's expectation that |
||||||||
|
|
||||||||
| def _gen_rows_with_counts(self) -> Iterator[Sequence[str]]: | ||||||||
| """Iterator with string representation of body data with counts.""" | ||||||||
|
|
||||||||
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.
This is a good addition to the bug fixes section. It clearly describes the issue and references the relevant GitHub issue. However, to improve clarity and consistency, we could make a small adjustment to the wording.