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

Allow InterProcessLock to receive a pathlib.Path object #16

Closed
cool-RR opened this issue Dec 6, 2015 · 8 comments
Closed

Allow InterProcessLock to receive a pathlib.Path object #16

cool-RR opened this issue Dec 6, 2015 · 8 comments

Comments

@cool-RR
Copy link

cool-RR commented Dec 6, 2015

No description provided.

@harlowja
Copy link
Owner

harlowja commented Dec 6, 2015

Sounds good to me as long as it will work on both py2.7 and py3.x

@cool-RR
Copy link
Author

cool-RR commented Dec 6, 2015

I think it will since you don't even need the package, just call str(path)
on the paths and you're done. You need pathlib just for the tests, and
there is a python 2 backport.
On Dec 7, 2015 01:10, "Joshua Harlow" notifications@github.com wrote:

Sounds good to me as long as it will work on both py2.7 and py3.x


Reply to this email directly or view it on GitHub
#16 (comment).

@harlowja
Copy link
Owner

harlowja commented Dec 7, 2015

Agreed, although probably better to call https://docs.python.org/3/library/os.html#os.fsencode if we can...

harlowja added a commit that referenced this issue Dec 7, 2015
Check and validate path types before further usage
and translate pathlib objects so that they can be
used correctly in further interprocess locking code.

Fixes issue #16
@harlowja
Copy link
Owner

harlowja commented Dec 7, 2015

Ok checkout #17 (I'll add some tests but seems to work locally).

harlowja added a commit that referenced this issue Dec 7, 2015
Check and validate path types before further usage
and translate pathlib objects so that they can be
used correctly in further interprocess locking code.

Fixes issue #16
@cool-RR
Copy link
Author

cool-RR commented Dec 7, 2015

Be wary of isinstance(path, pathlib.Path), because some people, like me,
bundle pathlib in another package so this check would fail for them. I
would skip the pathlib import completely, even though it's in try. You
can basically check if it's a string, and if it's not turn it into a string
in your favorite way.

On Mon, Dec 7, 2015 at 2:48 AM, Joshua Harlow notifications@github.com
wrote:

Ok checkout #17 #17 (I'll add
some tests but seems to work locally).


Reply to this email directly or view it on GitHub
#16 (comment).

@harlowja
Copy link
Owner

harlowja commented Dec 7, 2015

Hmmm, why u bundling it 😦

I get that concern, but it seems like a 1% case (and not especially a 1% case that I like, bundling stuff IMHO is ummm not helping the python ecosystem in general, although I get the reasons why people do it, I just don't agree with them, ha).

@cool-RR
Copy link
Author

cool-RR commented Dec 7, 2015

Also consider there are alternatives to Pathlib which act similarly but
will still fail your test. You can't isinstance all of them but you can do
str(path) which will work for all of them, I think.

Sent from my phone.
On Dec 7, 2015 9:17 PM, "Joshua Harlow" notifications@github.com wrote:

Hmmm, why u bundling it [image: 😦]

I get that concern, but it seems like a 1% case (and not especially a 1%
case that I like, bundling stuff IMHO is ummm not helping the python
ecosystem in general, although I get the reasons why people do it, I just
don't agree with them, ha).


Reply to this email directly or view it on GitHub
#16 (comment).

@harlowja
Copy link
Owner

harlowja commented Dec 7, 2015

Fair enough, maybe should just do str() as u said...

harlowja added a commit that referenced this issue Dec 10, 2015
Check and validate path types before further usage
and translate pathlib objects so that they can be
used correctly in further interprocess locking code.

Fixes issue #16
harlowja added a commit that referenced this issue Dec 10, 2015
Convert/normalize path types before further usage
and attempt to translate non-string objects so that
they can be used correctly in further interprocess
locking code.

Fixes issue #16
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

No branches or pull requests

2 participants