Annotate to_json #26449
Annotate to_json #26449
Conversation
pandas/io/common.py
Outdated
| @@ -67,7 +68,7 @@ def _is_url(url): | |||
| return False | |||
|
|
|||
|
|
|||
| def _expand_user(filepath_or_buffer): | |||
| def _expand_user(filepath_or_buffer: Any) -> Any: | |||
There was a problem hiding this comment.
Yea was thinking that too. My motivation for putting Any was that these methods truly do accept anything though and could just return the caller so it's a literal interpretation of that feature, though static typing would detect potential abuses.
Pushing up in next commit
Codecov Report
@@ Coverage Diff @@
## master #26449 +/- ##
===========================================
- Coverage 91.73% 41.69% -50.04%
===========================================
Files 174 174
Lines 50754 50758 +4
===========================================
- Hits 46560 21165 -25395
- Misses 4194 29593 +25399
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26449 +/- ##
==========================================
- Coverage 91.73% 91.73% -0.01%
==========================================
Files 174 174
Lines 50754 50758 +4
==========================================
+ Hits 46560 46561 +1
- Misses 4194 4197 +3
Continue to review full report at Codecov.
|
| if _PY_PATH_INSTALLED and isinstance(filepath_or_buffer, LocalPath): | ||
| return filepath_or_buffer.strpath | ||
| return _expand_user(filepath_or_buffer) |
There was a problem hiding this comment.
I had to refactor this because Mypy wasn't narrowing types at this point and still guessed that Path objects could be returned here based off of the signature of _expand_user even though the preceding isinstance check wouldn't allow that.
I commented on python/mypy#6847 (comment) so will see if that gets any attention, but the refactor works as well
| @@ -82,6 +87,8 @@ def _expand_user(filepath_or_buffer): | |||
| """ | |||
| if isinstance(filepath_or_buffer, str): | |||
| return os.path.expanduser(filepath_or_buffer) | |||
| elif isinstance(filepath_or_buffer, pathlib.Path): | |||
There was a problem hiding this comment.
We weren't expanding Path objects with existing code so this should handle that as well. Reviewing where to add test for this
There was a problem hiding this comment.
Might split this off into a separate issue/PR if I can find a bug using Path objects not expanding
|
Will come back to this at a later date |
Simple annotations as I was looking at the code
@gwrome and @vaibhavhrt