Repository navigation
Add __repr__ to TableProxy for meaningful print output - #194
Conversation
Co-authored-by: harumaki4649 <83683593+harumaki4649@users.noreply.github.com>
Co-authored-by: harumaki4649 <83683593+harumaki4649@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bandit found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
This PR addresses the issue where print(table_proxy) displayed an unhelpful default object representation (<builtins.TableProxy object at 0x...>) by implementing __repr__ and __str__ methods that show meaningful table contents in a dictionary-like format.
Key Changes
- Added
__repr__and__str__methods toTableProxyto display table name and contents - Added helper methods (
keys(),items(),__len__()) and repr methods toAsyncTableProxyfor feature parity - Comprehensive test suite covering basic, empty, multi-item, and async table proxy representations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| dictsqlite_v2/dictsqlite/src/lib.rs | Implements __repr__ and __str__ for TableProxy to format output as TableProxy('table_name', {key: value, ...}) |
| dictsqlite_v2/dictsqlite/src/async_ops.rs | Adds keys(), items(), __len__() helper methods and matching __repr__/__str__ implementations to AsyncTableProxy |
| dictsqlite_v2/dictsqlite/tests/test_table_proxy_repr.py | Comprehensive test suite with 5 test cases covering basic functionality, edge cases (empty tables), multiple items, async proxies, and print output verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// String representation: show table name and contents as dict | ||
| fn __repr__(&self, py: Python) -> PyResult<String> { | ||
| let items = self.items(py)?; | ||
| if items.is_empty() { | ||
| return Ok(format!("TableProxy('{}', {{}})", self.table_name)); | ||
| } | ||
|
|
||
| // Format items as dict-like string | ||
| let mut item_strs = Vec::new(); | ||
| for (key, value) in items { | ||
| // Try to get a string representation of the value using method chaining | ||
| let value_repr = value | ||
| .getattr(py, "__repr__") | ||
| .and_then(|repr_method| repr_method.call0(py)) | ||
| .and_then(|repr_result| repr_result.extract::<String>(py)) | ||
| .unwrap_or_else(|_| "...".to_string()); | ||
| item_strs.push(format!("{:?}: {}", key, value_repr)); | ||
| } | ||
|
|
||
| Ok(format!( | ||
| "TableProxy('{}', {{{}}})", | ||
| self.table_name, | ||
| item_strs.join(", ") |
There was a problem hiding this comment.
The __repr__ method loads all items from the table into memory by calling self.items(py)?. For large tables with thousands or millions of entries, this could cause significant performance degradation and memory issues when simply printing the table. Consider limiting the number of items displayed (e.g., first 10 items with "... N more items" indicator) or implementing a lazy representation that doesn't load all data upfront.
| /// String representation: show table name and contents as dict | |
| fn __repr__(&self, py: Python) -> PyResult<String> { | |
| let items = self.items(py)?; | |
| if items.is_empty() { | |
| return Ok(format!("TableProxy('{}', {{}})", self.table_name)); | |
| } | |
| // Format items as dict-like string | |
| let mut item_strs = Vec::new(); | |
| for (key, value) in items { | |
| // Try to get a string representation of the value using method chaining | |
| let value_repr = value | |
| .getattr(py, "__repr__") | |
| .and_then(|repr_method| repr_method.call0(py)) | |
| .and_then(|repr_result| repr_result.extract::<String>(py)) | |
| .unwrap_or_else(|_| "...".to_string()); | |
| item_strs.push(format!("{:?}: {}", key, value_repr)); | |
| } | |
| Ok(format!( | |
| "TableProxy('{}', {{{}}})", | |
| self.table_name, | |
| item_strs.join(", ") | |
| /// String representation: show table name and contents as dict (limited to first 10 items) | |
| fn __repr__(&self, py: Python) -> PyResult<String> { | |
| // Limit the number of items displayed | |
| const MAX_ITEMS: usize = 10; | |
| let keys = self.keys(py)?; | |
| let total = keys.len(); | |
| if total == 0 { | |
| return Ok(format!("TableProxy('{}', {{}})", self.table_name)); | |
| } | |
| // Get up to MAX_ITEMS keys | |
| let shown_keys = keys.iter().take(MAX_ITEMS); | |
| let mut item_strs = Vec::new(); | |
| for key in shown_keys { | |
| // Get value for key | |
| let value = self.get(py, key)?; | |
| let value_repr = match value { | |
| Some(val) => val | |
| .getattr(py, "__repr__") | |
| .and_then(|repr_method| repr_method.call0(py)) | |
| .and_then(|repr_result| repr_result.extract::<String>(py)) | |
| .unwrap_or_else(|_| "...".to_string()), | |
| None => "None".to_string(), | |
| }; | |
| item_strs.push(format!("{:?}: {}", key, value_repr)); | |
| } | |
| let more = if total > MAX_ITEMS { | |
| format!(", ... {} more items", total - MAX_ITEMS) | |
| } else { | |
| "".to_string() | |
| }; | |
| Ok(format!( | |
| "TableProxy('{}', {{{}{}}})", | |
| self.table_name, | |
| item_strs.join(", "), | |
| more |
| fn __repr__(&self, py: Python) -> PyResult<String> { | ||
| let items = self.items(py)?; | ||
| if items.is_empty() { | ||
| return Ok(format!("TableProxy('{}', {{}})", self.table_name)); | ||
| } | ||
|
|
||
| // Format items as dict-like string | ||
| let mut item_strs = Vec::new(); | ||
| for (key, value) in items { | ||
| // Try to get a string representation of the value using method chaining | ||
| let value_repr = value | ||
| .getattr(py, "__repr__") | ||
| .and_then(|repr_method| repr_method.call0(py)) | ||
| .and_then(|repr_result| repr_result.extract::<String>(py)) | ||
| .unwrap_or_else(|_| "...".to_string()); | ||
| item_strs.push(format!("{:?}: {}", key, value_repr)); | ||
| } | ||
|
|
||
| Ok(format!( | ||
| "TableProxy('{}', {{{}}})", | ||
| self.table_name, | ||
| item_strs.join(", ") | ||
| )) |
There was a problem hiding this comment.
The __repr__ implementation is duplicated identically in AsyncTableProxy. This duplication makes maintenance harder - if a bug is found or improvement is needed in one place, it must be updated in both. Consider extracting the formatting logic into a shared helper function that both implementations can use.
| for key in storage_keys { | ||
| if key.starts_with(&prefix) { | ||
| let short_key = key[prefix.len()..].to_string(); | ||
| if !all_keys.contains(&short_key) { | ||
| all_keys.push(short_key); |
There was a problem hiding this comment.
The loop uses all_keys.contains(&short_key) to check for duplicates, which is O(n) for each key from storage. For tables with many keys, this results in O(n²) complexity. Consider using a HashSet to track seen keys instead, which would reduce this to O(n):
use std::collections::HashSet;
let mut seen_keys = HashSet::new();
let mut all_keys: Vec<String> = db
.cache
.iter()
.filter(|entry| entry.key().starts_with(&prefix))
.map(|entry| {
let short_key = entry.key()[prefix.len()..].to_string();
seen_keys.insert(short_key.clone());
short_key
})
.collect();
// Then when checking storage:
if !seen_keys.contains(&short_key) {
all_keys.push(short_key.clone());
seen_keys.insert(short_key);
}| fn __repr__(&self, py: Python) -> PyResult<String> { | ||
| let items = self.items(py)?; | ||
| if items.is_empty() { | ||
| return Ok(format!("TableProxy('{}', {{}})", self.table_name)); | ||
| } | ||
|
|
||
| // Format items as dict-like string | ||
| let mut item_strs = Vec::new(); | ||
| for (key, value) in items { | ||
| // Try to get a string representation of the value using method chaining | ||
| let value_repr = value | ||
| .getattr(py, "__repr__") | ||
| .and_then(|repr_method| repr_method.call0(py)) | ||
| .and_then(|repr_result| repr_result.extract::<String>(py)) | ||
| .unwrap_or_else(|_| "...".to_string()); | ||
| item_strs.push(format!("{:?}: {}", key, value_repr)); | ||
| } | ||
|
|
||
| Ok(format!( | ||
| "TableProxy('{}', {{{}}})", | ||
| self.table_name, | ||
| item_strs.join(", ") | ||
| )) |
There was a problem hiding this comment.
Similar to the TableProxy implementation, the __repr__ method loads all items from the table by calling self.items(py)?, which could cause performance and memory issues for large tables. Consider limiting the number of items displayed or implementing a lazy representation.
Co-authored-by: harumaki4649 <83683593+harumaki4649@users.noreply.github.com>
Banditの警告を確認し、 日本語の報告書を
|
テーブル分離モードの実装について検討しました。 現在の実装では、テーブルはキーのプレフィックス（例： オプション1: SQLiteの別テーブルを使用
オプション2: 別ファイルを使用
この機能の実装には、Rust側の 実装をご希望の場合は、新しいIssueを作成していただければ対応いたします。 |
変更が必要なファイル
難易度: 中〜高理由:
不具合の発生可能性
複雑さの増加
推奨アプローチ段階的に実装することをお勧めします：
新しいIssueを作成していただければ、詳細な設計ドキュメントを作成できます。 |
printshows<builtins.TableProxy object at 0x...>instead of meaningful content__repr__method toTableProxyclass in Rust code (lib.rs)__repr__method toAsyncTableProxyclass in Rust code (async_ops.rs)__repr__functionalitySummary
This PR fixes the TableProxy print representation issue. Now when you print a TableProxy object, it shows:
Changes made:
__repr__and__str__methods toTableProxyclass inlib.rs__repr__,__str__,keys(),items(), and__len__()methods toAsyncTableProxyclass inasync_ops.rstest_table_proxy_repr.pyOriginal prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.