Conversation
- Changed Python version from 3.9 to 3.10 to support pattern matching syntax in pytest. - Added per-module options to disable arg-type errors for test files that use dict unpacking with TableSpec.
Missing imports: - Added logging import to commcare_export/excel_query.py. - Added explicit imports for test files (jsonpath, defaultdict, split_leftmost, etc.). - Added __all__ to commcare_export/__init__.py to explicitly export __version__. Type annotations: - Added type annotations for variables where mypy couldn't infer types: mappings, sheets_by_source, emits, depth, paginators. - Updated generic types to include parameters: list[Any], dict[str, int], etc. Property override issues: - Removed message = None from base DataExportException class and added it as a property. - Changed max_column_length in base TableWriter class to a property, in line with its definition in SqlMixin. - Added type annotations to BaseCommand attributes to allow both str and None. Other fixes: - Changed unknown_count from string 'unknown' to int -1 in commcare_hq_client.py. - Fixed tuple reassignment issue in misc.py by using a different variable name. - Added null check for archive.close() in writers.py. - Added assertions for non-None values in utils_cli.py. - Used CaseInsensitiveDict in test_commcare_hq_client.py. - Added type ignore comments for SQLAlchemy and setuptools compatibility.
Reflect that the `rows` attribute is an iterable not a list
millerdev
left a comment
millerdev
left a comment
There was a problem hiding this comment.
Few nitpicky comments, but I think nothing blocking.
| @property | ||
| def max_column_length(self): | ||
| return None |
There was a problem hiding this comment.
Would a simple assignment work?
| @property | |
| def max_column_length(self): | |
| return None | |
| max_column_length = None |
Edit: Oh, I see that was removed above, and the property was added. Why did it need to change?
There was a problem hiding this comment.
The SqlMixin class overloads it with a property (getter), and mypy complains when you overload a variable that you can set with one that you can't.
(The options were to (A) remove it from the superclass, (B) make it a property, or (C) add a setter to SqlMixin. (C) is unnecessary because the database engine determines the value, and (A) would be ugly because the code for managing query files (aka DET config files) expects TableWriter objects to have the attribute. So I went with (B).)
2 participants
Currently mypy checks type annotations in just 2 files. This change ...
🐡