remove hard-dependency on pyfilesystem2, add a minimal drop-in replacement #3885
Conversation
|
OSFS is just a thin wrapper around pathlib.Path, whereas ZipFS uses zipfile to open and read specific files within the archive (without unpacking it all); for the zip writer, it's just a TempFS that calls shutil.make_archive on close() (not dissimilar from the original fs.ZipFS). |
openbin -> open(mode=*b) so we have one fewer method to support; fs.path.parts[-1] is the same as fs.path.basename, prefer the latter
2f4a238 to
bb0b727
Compare
|
ok this is ready for review now. I tested ufoLib2 and defcon test suite and they just work with minor modification, notably stuff like - import fs
+ from fontTools.misc import filesystem as fs |
|
Where did you get all the implementation code from? Did you copy-paste and prune fs's code? |
|
well not literal copy paste, more like taking "inspiration" while aiming for the most concise approach to keep things working as before while using stdlib-only methods.. I could add citation links to original source code if you like. EDIT: I added original copyright and license notice to LICENSE.external file: 6580a42 |
|
No need, I just wondered how you hammered all that code out so quickly. |
|
Oh BTW, you may want to run |
| from fontTools.ufoLib.utils import numberTypes, _VersionTupleEnumMixin | ||
| from fontTools.ufoLib.filenames import userNameToFileName | ||
| from fontTools.ufoLib.utils import _VersionTupleEnumMixin, numberTypes | ||
| from fontTools.ufoLib.validators import * |
There was a problem hiding this comment.
Actually, maybe just import pngValidator?
There was a problem hiding this comment.
yeah ruff didn't like the star-import..
…yfilesystem2 API we care about
…thout it CI runs tox twice, the second time with -noextra which will not install any of the third-party deps listed in requirements.txt
... instead of try/except ImportError either upstream `fs` module or our replacement is used depending on whether the former is available
we currently don't use this (why would one ever want to *not* create dst_dir...) but at least I removed a TODO :)
This adds a new fontTools.misc.filesystem package that exposes the same API as pyfilesystem2 but only for the subset that fontTools.ufoLib uses: i.e. OSFS, SubFS, ZipFS, TempFS and other little helpers.
the requirement in setup.py was removed, so pip install fonttools[ufo] will no longer instal fs. However, the code continues to work with fs if one installs it separately and wishes to pass actual FS objects to ufoLib.
Similarly to what we do with lxml, I try to import fs and if that fails I fall back to this new fontTools.misc.filesystem. No other changes inside the fontTools.ufoLib code itself besides the try/import.
The CI runs test with and without fs. I think it's ok to continue supporting fs as a soft/optional dependency given that it still works (despite the pkg_resources deprecation warning), and it may be resurrected at some point.
But if one doesn't care about fancy fileststems but just UFO as local directories or zip, the fs module is no longer required basically and the warning shoud stop showing up.