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

Move to use pathlib instead of os.path and pathtools #556

Closed
rrzaripov opened this issue Mar 26, 2019 · 4 comments · Fixed by #704
Closed

Move to use pathlib instead of os.path and pathtools #556

rrzaripov opened this issue Mar 26, 2019 · 4 comments · Fixed by #704

Comments

@rrzaripov
Copy link
Contributor

This issue created for discuss about moving to use pathlib.
I have a next questions about this:

  • pathtools last updated in 2016, can pathlib replace it?
  • pathlib is a part of CPython from version 3.4 and available for version 2.7 as additional package (but 2.7 support dropping in next year, so this is not so critical). And if answer on the first question is yes, we decrease count of dependencies.
  • in pathlib path is a object, not a string as in os.path. No more think about encodings, it is "bytes" or "str", os.path.sep, startswith and etc.

Also, I want to know, maybe any potential problems for moving to pathlib? What youre think about this?

@chrahunt
Copy link

We should also check pathlib2, which is a more recently updated version of the same.

@bstaletic
Copy link
Contributor

Watchdog uses only three things from pathtools:

  1. absolute_path() which does os.path.abspath(os.path.normpath()). Can be replaced with Path.resolve().
  2. filter_paths() in the tests and match_any_paths() in events.py.

Here's how the latter two look like done with pathlib:

from pathlib import PureWindowsPath, PurePosixPath

def _match_path(path, included_patterns, excluded_patterns):
  common_patterns = included_patterns & excluded_patterns
  if common_patterns:
    raise ValueError(f'conflicting patterns `{common_patterns}` included and excluded')
  return (any(path.match(p) for p in included_patterns) and
          not any(path.match(p) for p in excluded_patterns))


def filter_paths(paths, included_patterns = None, excluded_patterns = None, case_sensitive = True):
  included = ["*"] if included_patterns is None else included_patterns
  excluded = [] if excluded_patterns is None else excluded_patterns

  for path in paths:
    p = PurePosixPath(path) if case_sensitive else PureWindowsPath(path)
    if _match_path(p, set(included), set(excluded), case_sensitive):
      yield p


def match_any_paths(paths, included_patterns = None, excluded_patterns = None, case_sensitive = True):
  included = ["*"] if included_patterns is None else included_patterns
  excluded = [] if excluded_patterns is None else excluded_patterns

  for path in paths:
    p = PurePosixPath(path) if case_sensitive else PureWindowsPath(path)
    if _match_path(p, set(included), set(excluded), case_sensitive):
      yield p

As for the os.path module:

os.path pathlib.Path
join() Just pass arguments to the constructor
exists() exists()
isdir() is_dir()
islink() is_symlink()
dirname() parent

After that, there would only be two cases of startswith and two cases of endswith.

@bstaletic
Copy link
Contributor

Here's my first attempt at switching to pathlib: bstaletic@35feb63

If there's actual interest, I can make a pull request.

@BoboTiG
Copy link
Collaborator

BoboTiG commented Nov 27, 2020

Nice! Go ahead for a PR, we just dropped the Python 2.7 support so your changes will work directly.

Think to "backport" docstrings and doctests from the original pathtools source code.

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

Successfully merging a pull request may close this issue.

4 participants