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

py.path vs pathlib.Path #7

Closed
neighthan opened this issue Feb 21, 2019 · 6 comments · Fixed by #21
Closed

py.path vs pathlib.Path #7

neighthan opened this issue Feb 21, 2019 · 6 comments · Fixed by #21

Comments

@neighthan
Copy link

It seems that the datafiles fixture is a py.path (well, a LocalPath) - does this have any functionality that pathlib.Path does not? If not, I think using pathlib's Path would be nice. The only function I've missed so far is is_file, but I haven't tested this much. Additionally, the py website says

Note: The ‘py’ library is in “maintenance mode” and so is not recommended for new projects. Please check out pathlib or pathlib2 for path operations.

@omarkohl
Copy link
Owner

omarkohl commented Feb 26, 2019

The reason is that the tmpdir fixture which is provided out of the box is a py.path.local object... But I see that since pytest version 3.9 there is a new tmp_path fixture which is a pathlib/pathlib2.Path object and as far as I can tell it does exactly the same as tmpdir so I see no problem with updating this plugin. Would you like to submit a pull request?

https://docs.pytest.org/en/latest/tmpdir.html
pytest-dev/pytest#3985

@neighthan
Copy link
Author

While working on a PR for this, I found several differences between the methods of pathlib.Path and py.path (e.g. py.path has isdir instead of is_dir, mksymlinkto instead of symlinkto, isdir instead of is_dir; py.path also has copy which pathlib.Path doesn't have at all (I usually do target.write_bytes(src.read_byes())). So, for the sake of backwards compatibility, I don't think anymore that it would be good to return a pathlib.Path instead; there are too many ways that somebody's code could break. If anybody does want a pathlib.Path, it's easy to get one: pathlib.Path(str(datafiles)).

@dazza-codes
Copy link

https://py.readthedocs.io/en/latest/path.html is in maintenance mode and recommends moving to pathlib.Path - at some point this could break somewhere in upstream dependencies. I might be useful to leave this issue open.

@jlumbroso
Copy link

@omarkohl Do you want (and would you accept a PR) that moves from py to pathlib, as per pytest? I note that you've already done BREAKING CHANGES in the past, and have adjusted the major version, as in the changes suggested by @tomjnixon.

I have been looking for an elegant way to use data files in my tests, and your library looks like it would do the trick perfectly, but I don't want to start using a dependency that has explicitly said it is in maintenance mode.

Let me know!

@radusuciu
Copy link

radusuciu commented Jun 30, 2022

I would also like to see this but honestly it's mostly for aesthetic reasons (so long as the py module doesn't get deprecated in either python or pytest), so I can understand why it may not be desired and sympathize with the message from the README: "There is little activity because it simply works and no changes are required."

There is a fork, and this commit in particular: 9c42adf that implements this, but I haven't tested it.

@omarkohl
Copy link
Owner

If someone wants to submit a pull request it could be released as a major version breaking change.

@omarkohl omarkohl reopened this Jun 30, 2022
hans-d added a commit to hans-d/pytest-datafiles that referenced this issue Feb 18, 2023
@hans-d hans-d mentioned this issue Feb 18, 2023
hans-d added a commit to hans-d/pytest-datafiles that referenced this issue Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants