Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

additional_skip_names are not respected everywhere in pathlib #1023

Closed
sassanh opened this issue May 27, 2024 · 4 comments
Closed

additional_skip_names are not respected everywhere in pathlib #1023

sassanh opened this issue May 27, 2024 · 4 comments
Labels

Comments

@sassanh
Copy link
Contributor

sassanh commented May 27, 2024

Describe the bug

Path().exists and Path().unlink for example don't work well with additional_skip_names.

Context: #1021 (comment)

The current implementation is incomplete (yes, actually all methods should respect additional_skip_names) and ugly (using the callstack). I had hoped that it will sufficient until I get back to it, but obviously I was wrong.

@sassanh
Copy link
Contributor Author

sassanh commented May 27, 2024

and ugly (using the callstack)

I would probably implement it by checking the call stack too, as the final solution.
Actually I think checking the call stack is better than module patching, because a Path object may be imported in another module (not listed in skipped modules) and passed to as an argument to a function in this module, module patching wouldn't work well and one would require hacks like this: https://github.com/sassanh/python-redux/blob/main/redux_pytest/fixtures/snapshot.py#L92

Do you have a scenario in head for which checking call stack would cause trouble/unexpected behavior.

@mrbean-bremen
Copy link
Member

Do you have a scenario in head for which checking call stack would cause trouble/unexpected behavior.

Probably not. I just didn't like it much, but you may be right.

@mrbean-bremen
Copy link
Member

I had that (using the callstack) in mind to use for all functions, but thought that there should be a better way. But as I wrote, this didn't work out. So, you are probably right that this is the way to go - I was just reluctant as I find it somewhat hacky. And with the callstack, you could just do it a single place, so it would probably be ok.

I would probably implement it by checking the call stack too, as the final solution.

So, you are going to make another PR? Would be awesome!

@sassanh
Copy link
Contributor Author

sassanh commented May 27, 2024

So, you are going to make another PR? Would be awesome!

I will try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants