support python to_dataframe. - #624
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a utility function to_dataframe() to convert TsFile data into pandas DataFrames, along with supporting error handling infrastructure. The implementation allows users to read entire tables or specific columns from TsFiles with optional row limits.
Key changes:
- New
to_dataframe()utility function for reading TsFile data into pandas DataFrames - Addition of
ColumnNotExistErrorexception class and error mapping - Helper method
get_column_names()inTableSchemaclass - Comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| python/tsfile/utils.py | New utility module containing the to_dataframe() function for converting TsFile data to pandas DataFrame |
| python/tsfile/schema.py | Added get_column_names() method to TableSchema class for retrieving column names |
| python/tsfile/exceptions.py | Added ColumnNotExistError exception class, updated error mapping, and reformatted spacing |
| python/tsfile/init.py | Exported to_dataframe function for public API |
| python/tests/test_write_and_read.py | Added comprehensive test coverage for to_dataframe() functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with pytest.raises(TableNotExistError): | ||
| df2 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_tb") | ||
| with pytest.raises(ColumnNotExistError): | ||
| df3 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", "device1") |
There was a problem hiding this comment.
The test passes a string 'device1' for the column_names parameter, but the function signature expects list[str]. This should be ['device1'] (a list) to match the API design. The test will fail unless the implementation handles this type mismatch, which it doesn't.
| df3 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", "device1") | |
| df3 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", ["device1"]) |
| total_rows += batch_rows | ||
| else: | ||
| df = result.read_data_frame() | ||
| df_list.append(df) |
There was a problem hiding this comment.
If no data is read (empty result set), df_list will be empty and pd.concat() will raise a ValueError. Consider adding a check: if not df_list: return pd.DataFrame() before the concat operation.
| df_list.append(df) | |
| df_list.append(df) | |
| if not df_list: | |
| return pd.DataFrame(columns=column_names) |
| if remaining_rows <= 0: | ||
| break | ||
| else: | ||
| batch_rows = min(remaining_rows, 1024) |
There was a problem hiding this comment.
The magic number 1024 for batch size is hardcoded. Consider defining it as a named constant at the module level (e.g., BATCH_SIZE = 1024) to improve maintainability and make it easier to adjust.
| else: | ||
| batch_rows = min(remaining_rows, 1024) | ||
| df = result.read_data_frame(batch_rows) | ||
| total_rows += batch_rows |
There was a problem hiding this comment.
The total_rows counter is incremented by batch_rows (the requested rows), but this assumes all requested rows were successfully read. If the result set has fewer rows than batch_rows, this will cause an inaccurate count and potentially allow more rows to be read than max_row_num. Consider using total_rows += len(df) instead.
| total_rows += batch_rows | |
| total_rows += len(df) |
| assert df2.shape == (4097, 3) | ||
| assert df1["value2"].equals(df2["value2"]) | ||
| with pytest.raises(TableNotExistError): | ||
| df2 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_tb") |
There was a problem hiding this comment.
Variable df2 is not used.
| df2 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_tb") | |
| tsfile.to_dataframe("table_write_to_df.tsfile", "test_tb") |
| with pytest.raises(TableNotExistError): | ||
| df2 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_tb") | ||
| with pytest.raises(ColumnNotExistError): | ||
| df3 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", "device1") |
There was a problem hiding this comment.
Variable df3 is not used.
| df3 = tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", "device1") | |
| tsfile.to_dataframe("table_write_to_df.tsfile", "test_table", "device1") |
| import pytest | ||
|
|
||
| from tsfile import ColumnSchema, TableSchema, TSEncoding, NotSupportedError | ||
| import tsfile |
There was a problem hiding this comment.
Module 'tsfile' is imported with both 'import' and 'import from'.
| import tsfile |
| from tsfile.exceptions import * | ||
|
|
||
|
|
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module tsfile.exceptions does not define 'all'.
| from tsfile.exceptions import * |
|
|
||
| import pandas as pd | ||
|
|
||
| from tsfile.exceptions import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module tsfile.exceptions does not define 'all'.
| from tsfile.exceptions import * | |
| from tsfile.exceptions import TableNotExistError, ColumnNotExistError |
There was a problem hiding this comment.
Use explicit imports instead of wildcards (*)
|
|
||
| import pandas as pd | ||
|
|
||
| from tsfile.exceptions import * |
There was a problem hiding this comment.
Use explicit imports instead of wildcards (*)
No description provided.