Allow users to explicitly specify path module (e.g. posixpath) #9

Merged
merged 2 commits into from Nov 13, 2012

Projects

None yet

2 participants

@vstojkovic

This change allows users to specify an alternate path module instead of os.path on a per-instance basis, as we discussed in issue #8.

@vstojkovic

I'm sorry, I didn't know that a pull request would open its own, duplicate issue. Is there a way I can fix this mess?

@jaraco
Owner
jaraco commented Nov 13, 2012

Don't worry about it. Apparently, it's possible to convert an issue into a pull request using their API (http://opensoul.org/blog/archives/2012/11/09/convert-a-github-issue-into-a-pull-request/), but some say they don't even recommend doing that. Having the issue and pull request tracked separately has its advantages.

@jaraco
Owner
jaraco commented Nov 13, 2012

One comment - when the new paths are created, should they be created using the same 'module'? i.e.:

def abspath(self):       return self.__class__(self.module.abspath(self), self.module)

Surely, we need a test to ensure that the following produces paths with consistent, predictable path separators:

path('foo', posixpath) / 'bar' / 'baz'
path('foo', ntpath) / 'bar' / 'baz'

At this point, I start to wonder if it makes sense instead to use a class attribute, i.e.:

class path(unicode):
    module = os.path

Then a subclass could override the module:

class WinPath(path):
    module = ntpath

or an application that doesn't care about affecting all path instances could simply replace the class attribute for all path instances:

# default to posixpath for all instances
path.module = posixpath

It also means that the constructor signature doesn't change, which is a huge boon in terms of avoiding conflicts in the future.

It does complicate the basic usage, however, where one wants to construct an explicit path using an explicit module. For that, I see a couple of options. One option, a class constructor that simply sets the module attribute:

@classmethod
def using_module(cls, source, module=None):
    path_ob = cls(source)
    if module: path_ob.module = module

Then, one could construct a simple path with customized module as so:

my_path = path.using_module('foo', module=posixpath)

However, that path's methods would end up returning paths of the default module. Perhaps instead, using_module should create a new class whose module is customized:

@classmethod
def using_module(cls, module):
    cls_name = cls.__name__ + '_' + module.__name__
    bases = (path,)
    ns = {'module': module}
    return type(cls_name, bases, ns)

Then, one could construct a path with a customized module as so:

my_path = path.using_module(posixpath)('foo')

We could even create handy aliases for the two common use-cases:

posix_path = path.using_module(posixpath)
nt_path = path.using_module(ntpath)

Although this last implementation may seem the most complicated at first blush, I like it the most because it's the least intrusive and the most flexible.

@vstojkovic

Oops, I dropped the ball there. I should have included the code that passes module reference in every method that creates a new path.

I guess that just underlines what you said: adding a constructor argument complicates things more than having it in the class.

I didn't think of creating classes dynamically. I'll try the approach you described at the end.

@jaraco
Owner
jaraco commented on d36450e Nov 13, 2012

I like it. I think I'd like to omit the value argument to using_module, as the caller can just as easily construct an instance with a value from the class that's returned, and it's not obvious which method would be preferable. All in all, great work!

@jaraco jaraco merged commit d36450e into jaraco:master Nov 13, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment