♻️ Re-design package and introduce large parts of the API #90
Conversation
|
|
|
Still need to add docs. |
falexwolf
changed the title
|
This is all great work, Sergei! |
nbproject/_dev/_dependency.py
Outdated
| def infer_dependencies(content: Union[Notebook, list], pin_versions: bool = True): | ||
| """Parse the notebook content and infer all dependencies. | ||
|
|
||
| Params |
There was a problem hiding this comment.
Sergei, I'm sorry for not having clearly communicated this before. We're doing Google-style docstrings across all packages. With the advent of type annotations in the signatures, Google's more compact format works better. I'm very happy to convert all of the numpy-style docstrings to Google-style but I don't want to interfere with your PR. 😅
There was a problem hiding this comment.
The changes only affect the param-lists, which are present in about 4 docstrings.
There was a problem hiding this comment.
Ok, i will change, but i am pretty sure i saw numpy style in other repos.
There was a problem hiding this comment.
Yes, we started out with numpy-style, but changed about a month ago. Besides looking more compact while remaining readable, Google-style is also faster to write! Hence that decision.
falexwolf
changed the title
falexwolf
changed the title
|
Sorry for the slow response! Yes, your last solution seems the right way to do it! Document an un-initialized class I've used that strategy many times. |
nbproject/__init__.py
Outdated
| @@ -32,7 +31,7 @@ | |||
| __version__ = "0.0.9" | |||
|
|
|||
| from ._header import Header # noqa | |||
| from ._meta import Live, Meta, Metadata | |||
There was a problem hiding this comment.
What is Metadata as opposed to Meta? 🤔
We didn't go this granular in the design specification and you also don't explain it in the PR description.
There was a problem hiding this comment.
Yes, sorry, meta is an object of the class Meta. This object is initialized on import with the filename, time_run of the current notebook. And Metadata is just a pydantic model for nbproject.metadata. Is the name confusing?
There was a problem hiding this comment.
It's confusing that there is both Meta and Metadata.
I now understand that the latter is a mere shallow wrapper of the notebook metadata itself.
Hence, Meta is the nbproject metadata, whereas Metadata is the nbformat metadata.
There was a problem hiding this comment.
I assume the typical user won't see Metadata, so we could give this a longer name to disambiguate it for the developers.
There was a problem hiding this comment.
How about MetaJSON or MetaFormat or JSONMeta or NBFormatMeta. Or just everything lowercase for simplicity?
There was a problem hiding this comment.
All the variants look ugly to me. NBProjectMetadata maybe? Or MetaStore?
There was a problem hiding this comment.
MetaStore is great if that's what is returned in Meta.store!
There was a problem hiding this comment.
It's the perfect name for this!
|
In the commit, I'm making a mere suggestion on listing all classes on one page instead of generating page stubs. The page stubs are great for complex classes with many attributes and methods. The classes here are not so complicated and the user can probably parse the design most rapidly if it's on a single page. |
nbproject/_dev/_initialize.py
Outdated
| from ._notebook import Notebook | ||
|
|
||
|
|
||
| class Metadata(BaseModel): |
There was a problem hiding this comment.
Another good name could be MetaBase or MetaRaw, as that would suggest that it's upstream of nbproject.Meta.
There was a problem hiding this comment.
When you do the rename to MetaStore, you can also add a docstring.
"""The metadata stored in the notebook file ."""
Or something like this.
There was a problem hiding this comment.
The documentation will automatically show that the base class is pydantic.BaseModel.
nbproject/_meta.py
Outdated
|
|
||
| nb_meta = read_notebook(filepath).metadata | ||
| if "nbproject" in nb_meta: | ||
| self._store = Metadata(**nb_meta["nbproject"]) |
There was a problem hiding this comment.
Yes, exactly! MetaStore is the perfect name here! 🤩
nbproject/_meta.py
Outdated
| return self._live | ||
|
|
||
| @property | ||
| def store(self): |
There was a problem hiding this comment.
You could then add a return type MetaStore here.
nbproject/_meta.py
Outdated
| if filepath is None: | ||
| filepath = notebook_path() | ||
|
|
||
| self._live = Live(filepath, time_run) |
There was a problem hiding this comment.
I'd suggest you rename Live to MetaLive and then everything will be named in a parallel fashion.
nbproject/_meta.py
Outdated
| self._store = None | ||
|
|
||
| @property | ||
| def live(self): |
There was a problem hiding this comment.
And then you can add a return type MetaLive and it should render beautifully in the docs!
|
Awesome! This looks super great now! 😅 |
|
Would you want to merge and @sunnyosun & I test it on main? |
|
@falexwolf i am merging this. |
| from ._header import _filepath, _time_run | ||
| from ._logger import logger | ||
|
|
||
|
|
||
| def get_title(nb: Notebook) -> Union[str, None]: |
There was a problem hiding this comment.
There was a problem hiding this comment.
On the other hand, it is displayed as Optional in the docs anyways, even if written as Union.
There was a problem hiding this comment.
I know! But that's going to go away with Python 3.10. We'll have beautiful str | None then.
|
Thank you, @Koncopd! Are you down for celebrating this? Getting a beer somewhere in the next few days or so? |
|
@falexwolf yep, why not, Friday? |
This PR largely follows the design spec here: https://www.notion.so/laminlabs/50e2e172242b4c609f600f6a282831c2.
Summary:
Added developer API to
nbproject.devwith:infer_dependenciesfor dependencies inference from a notebook.initialize_metadatafor nbproject metadata initialization.check_integrityfor integrity check: Add an integrity check functionality #70read_notebook,write_notebook(along with the notebook modelNotebook) for jupyter notebook io.notebook_paththere.Added testing API
nbproject.dev.testwithexecute_notebooks, which executes all notebooks in a given folder.Added metadata API to
nbproject.metawithstorea wrapper aroundnb.metadata["nbproject"].liveAPI-level access to metadata computed at run-time with.time_run(current date & time),.time_passed(compute time spent within the notebook),.integrity(current integrity status of the current notebook),.dependency(dependencies inferred from current notebook content & environment).Notes:
nbproject.devis structured to avoid loading heavy modules (_dependency.py) on init and avoid the need to add things to dev__init__.pyas it is impossible to load without executing__init__.py. It is possible, for example, to dofrom nbproject.dev import test; test.execute_notebooks(), it is impossible to dofrom nbproject.dev.test import execute_notebooks.