-
Notifications
You must be signed in to change notification settings - Fork 107
Description
Summary
Several converter classes have convert() methods that contain only pass, returning None unexpectedly. While these converters are not typically invoked through the normal code path (since pandas/arrow/polars result sets read from S3 files using their own type systems), the pass stub can cause confusion and unexpected behavior if the method is called (e.g., via the API fallback path in _fetch_all_rows).
Affected classes
| Class | File | Has pass stub |
|---|---|---|
DefaultPandasTypeConverter |
pyathena/pandas/converter.py |
Yes |
DefaultPandasUnloadTypeConverter |
pyathena/pandas/converter.py |
Yes |
DefaultArrowUnloadTypeConverter |
pyathena/arrow/converter.py |
Yes |
DefaultPolarsUnloadTypeConverter |
pyathena/polars/converter.py |
Yes |
Note: DefaultArrowTypeConverter and DefaultPolarsTypeConverter already have proper implementations (converter = self.get(type_); return converter(value)).
Proposal
Delegate to DefaultTypeConverter.convert() for the pass-stub classes. This is the safest approach — if the method is ever called (e.g., via _fetch_all_rows API fallback), it will produce correct results instead of silently returning None.
Definition of Done
- All four
passstubs replaced with workingconvert()implementations - No behavior change for the normal S3-based read path
- API fallback path (
_fetch_all_rows) works correctly with these converters
Context
Identified during PR review of result_set_type_hints feature (#690). The _fetch_all_rows fallback path already defaults to DefaultTypeConverter() when no converter is provided, which works around this issue. However, the pass stubs remain a source of potential confusion.