Resolve frame paths in was_called_by (#3536)#3538
Resolve frame paths in was_called_by (#3536)#3538hyfc wants to merge 11 commits intopython-telegram-bot:masterfrom
was_called_by (#3536)#3538Conversation
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the quick PR! Nice that you also managed to write a test case for it 👍 I have a few comments about that:
- Could you replace the
oscalls by the correspondingpathlibequivalents? - I'm wondering if pytest could help with creating temporary directories: https://docs.pytest.org/en/6.2.x/tmpdir.html. that could make it possible to remove the custom cleanup logic. Plus, if the temporary directories/files are not created in the test directory, there no risk of producing left over artifacts ...
- IISC your test case currently cover only one of the changes lines. Maybe it's possible to cover also the second one?
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thank you for the updates! I have some more nitpicking, but nitpicking is really all it is :)
|
btw, don't worry about the faild |
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
|
Looks like adding Haven't tested it on Windows, but if it breaks, I'm ok to drop this PR. We can live with a minor warning message:sweat_smile: . |
|
That's unfortunate :/ I'll try to have a closer look at it soon™, but I agree that the priority on this may not be too high :D |
|
Not able to reproduce the failed |
|
FYI: I tried around some more in #3552 and arrived at a solution that I'm happy with. |
Fixes #3536 which causes false warning message during bot initialization. Now
telegram.ext._utils.stack.was_called_bywill always resolve paths in frame before comparing them with caller path.Checklist for PRs
Added.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)Added myself alphabetically toAUTHORS.rst(optional)Added new classes & modules to the docs and all suitable__all__s