[WIP] Create GISDocuments as untitled when no path provided
#630
Conversation
…s()` We now create untitled documents when no path is passed in, so users can use the JupyterLab facilities to rename the document.
|
|
| @@ -46,8 +46,6 @@ class GISDocument(CommWidget): | |||
| :param path: the path to the file that you would like to open. If not provided, a new empty document will be created. | |||
| """ | |||
|
|
|||
| path: Optional[Path] | |||
|
|
|||
There was a problem hiding this comment.
We don't know the path when we create an untitled document; it could be untitled.jGIS or untitled99.jGIS. The Javascript side knows, but how do we get that info into Python land? See #629
There was a problem hiding this comment.
Maybe we should require the path argument in the GISDocument, to avoid
- adding a way to display the path of the document in UI
- adding a way to change it (
save as), which would lead to the same issue as in Addsave_as()method to GISDocument API #595
There was a problem hiding this comment.
Otherwise the untitled.jGIS should probably be created from the kernel itself.
There was a problem hiding this comment.
Agreed, that's also what I commented in #629
|
|
||
| self.path = path | ||
|
|
||
| comm_metadata = GISDocument._path_to_comm(str(self.path) if self.path else None) |
There was a problem hiding this comment.
We're only using this variable in one place.
| comm_metadata=dict(ymodel_name="@jupytergis:widget", **comm_metadata), | ||
| comm_metadata={ | ||
| "ymodel_name": "@jupytergis:widget", | ||
| **self._path_to_comm(path), |
There was a problem hiding this comment.
Instance methods can call class methods from self, which is less brittle than calling it from GISDocument.
There was a problem hiding this comment.
Changing from dict() to {} was just personal preference. No strong feeling.
mfisher87
force-pushed
the
untitled-documents-by-default
branch
from
27b0739 to
a7ca246
Compare
|
Integration tests report: appsharing.space |
| assert (tmp_path / fn).is_file() | ||
| GISDocument() | ||
| assert len(list(tmp_path.iterdir())) == 2 | ||
| assert (tmp_path / "untitled1.jGIS").is_file() |
There was a problem hiding this comment.
This seemed like the natural way to test this, but I believe because there is no JupyterLab instance running, the JavaScript code which actually creates the files is not being exercised. Is there a better way to test this? Please advise :)
|
Lite tests are timing out again! |
mfisher87
changed the title
GISDocuments as untitled when no path providedGISDocuments as untitled when no path provided
|
Bug: It creates the file, but it's empty and changes to the widget to not update the file. |
|
You need to add default values to |
|
Thanks Trung! I'm going to move this one back to a draft as I think we don't have consensus that this behavior is what we want. |
Description
Always back
GISDocuments with files -- it's a document, after all!As pointed out by @fperez having the document sometimes be file-backed and other times not file-backed could be confusing to users. Fernando drew comparison between Notebooks and consoles; Notebooks are obviously documents, and consoles are obviously ephemeral. They tell you this from visual context. Presenting an identical interface for a document and ephemeral environment will likely confuse some users.
@brichet proposes a way forward to explicitly create an ephemeral environment with visual indicators here: #596 (comment)
Reverts #595 : Because we are now creating untitled documents when no path is passed (i.e.
GISDocument()), users can now use the JupyterLab facilities to rename their documents.Checklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lint