pyfilesystem2 icon indicating copy to clipboard operation
pyfilesystem2 copied to clipboard

update example in README

Open vpoulailleau opened this issue 3 years ago • 6 comments

Currently, the example count_python_loc is misleading, the normal Python version is way easier than what is suggested, at least if you don't consider Python versions that are end of life (https://devguide.python.org/#status-of-python-branches and https://devguide.python.org/devcycle/#end-of-life-branches).

The normal version should be something like:

from pathlib import Path

def count_python_loc(path):
    """Count non-blank lines of Python code."""
    return sum(
         sum(1 for line in file.read_text() if line.strip())
         for file in Path(path).rglob("*.py"))

To my mind, this should be reflected in the documentation. And maybe a read_text and a read_bytes method equivalent could be added. What's your opinion on this?

vpoulailleau avatar May 03 '22 06:05 vpoulailleau

I agree that it might make sense to use "more modern" Python in the example, but that probably can't happen until after #317 has been resolved?

lurch avatar May 06 '22 09:05 lurch

@lurch Thanks for the info.

I agree with the last comment in the mentioned issue (https://github.com/PyFilesystem/pyfilesystem2/issues/317#issuecomment-1119445955), and yes, to my mind, #317 should be closed first (or simultaneously with this issue).

vpoulailleau avatar May 06 '22 10:05 vpoulailleau

I suppose we could have both, pathlib in available in all Python3 versions nowadays, but I still find myself using os.path out of habit so I'd rather still have the current example as well.

althonos avatar May 06 '22 10:05 althonos

I quite disagree, according to PEP 20: "There should be one-- and preferably only one --obvious way to do it."

We should choose between os.path and pathlib.

According to https://docs.python.org/3/library/pathlib.html ("For low-level path manipulation on strings, you can also use the os.path module.") I would vote for pathlib as higher level lib (os.path may be needed in specific cases for small performance gains)

But I let the final decision to the project contributors.

vpoulailleau avatar May 06 '22 10:05 vpoulailleau

More on this:

  • https://treyhunner.com/2018/12/why-you-should-be-using-pathlib/
  • https://treyhunner.com/2019/01/no-really-pathlib-is-great/ (talking about the performance issue)

vpoulailleau avatar May 06 '22 10:05 vpoulailleau

I've just given this a bit more thought... the pyfilesystem API is broadly modelled around the old os.path API (as that was all that existed when pyfilesystem was first created); so perhaps it doesn't really make sense to switch the non-pyfilesystem-example to use pathlib, until / unless pyfilesystem also has a more pathlib-esque API? :shrug: (see #238 )

lurch avatar May 07 '22 10:05 lurch