overhaul packaging and project layout, bring in line with templates, first pass for fsspec #231
Conversation
painebot
force-pushed
the
tkp/overhaul
branch
6 times, most recently
from
8880bf6 to
a227e68
Compare
|
browsing s3 with s3fs 
|
painebot
force-pushed
the
tkp/overhaul
branch
3 times, most recently
from
f4b6b9e to
5b166cf
Compare
|
|
painebot
force-pushed
the
tkp/overhaul
branch
from
5b166cf to
7bfa46b
Compare
painebot
force-pushed
the
tkp/overhaul
branch
from
7bfa46b to
587cc60
Compare
painebot
force-pushed
the
tkp/overhaul
branch
2 times, most recently
from
a1569db to
9007937
Compare
first pass for fsspec
painebot
force-pushed
the
tkp/overhaul
branch
from
9007937 to
3a0b0c4
Compare
There was a problem hiding this comment.
Thanks @timkpaine ! I have some small fixes in a PR that I will submit shortly, but I also made some comments/notes for some parts of this that I'm not 100% sure on. Would appreciate if you had a look.
| "type": { | ||
| "description": "Backend type that the snippet works with", | ||
| "type": "string", | ||
| "default": "" |
There was a problem hiding this comment.
Should this also be an enum?
There was a problem hiding this comment.
yep i think so, i have validation on the backend
|
|
||
| self.resources = [] | ||
| self._default_root_manager = self._jupyterfsConfig.root_manager_class(**self._kwargs) | ||
| self._managers = dict((("", self._default_root_manager),)) | ||
|
|
||
| # copy kwargs to pyfs_kw, removing kwargs not relevant to pyfs | ||
| self._pyfs_kw.update(kwargs) | ||
| for k in (k for k in ("config", "log", "parent") if k in self._pyfs_kw): | ||
| self._pyfs_kw.pop(k) |
There was a problem hiding this comment.
I think we need to retain this behavior to ensure traitlets kwargs gets passed correctly to the child-managers for all possible config routes.
There was a problem hiding this comment.
We can add it back in, we may need to separate pyfs_kw and fsspec_kw
| elif resource["type"] == "fsspec": | ||
| manager_type = FSSpecManager | ||
|
|
||
| managers[_hash] = manager_type.create( | ||
| urlSubbed, | ||
| default_writable=default_writable, | ||
| parent=self, | ||
| **self._pyfs_kw, |
There was a problem hiding this comment.
This variable should depend on type.
|
thanks for the review @vidartf! I will wait for your reviews, i wasn't sure if you had time to look and didn't want to impose |
Yeah, no worries! As long as releases aren't rushed out I can always post-review like this (although that is less convenient). And if I'm offline for a while I shouldn't block everything either 😃 |
This PR does a few things.
fsspec support
fixes #102 (xref #222)
adapts @martindurant's code from main...martindurant:changes
top is pyfilesystem, buttom is fsspec
move to @tree-finder
fixes #185
Moves from
tree-finderto@tree-finder/basewhich has some fixes and is actively developedadjust snippets for fsspec support
Tweaks snippets to allow for fsspec, adjusts some text. Allows for backend-specific snippets (by
typefsspec/pyfsand also url e.g.local://).pyfs
fsspec
move to centralized template
http://github.com/python-project-templates/base
Easier to support from one location
move to pnpm
faster, better, standard with template
🚨 this pr *should* be backwards-compatible with the current code (all defaults set to pyfilesystem), but we should bump a minor version