search

harlowja / fasteners

A python package that provides useful locks.
Apache License 2.0
246 stars 45 forks source link

Allow `InterProcessLock` to receive a `pathlib.Path` object #16

Closed cool-RR closed 8 years ago

harlowja commented 8 years ago

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

cool-RR commented 8 years ago

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 https://github.com/harlowja/fasteners/issues/16#issuecomment-162363792.

harlowja commented 8 years ago

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

harlowja commented 8 years ago

Ok checkout https://github.com/harlowja/fasteners/pull/17 (I'll add some tests but seems to work locally).

cool-RR commented 8 years ago

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 https://github.com/harlowja/fasteners/pull/17 (I'll add some tests but seems to work locally).

— Reply to this email directly or view it on GitHub https://github.com/harlowja/fasteners/issues/16#issuecomment-162381813.

harlowja commented 8 years ago

Hmmm, why u bundling it :frowning:

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 commented 8 years ago

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: :frowning:]

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 https://github.com/harlowja/fasteners/issues/16#issuecomment-162629317.

harlowja commented 8 years ago

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