Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add notebook_dir as trait to be deprecated #36

Merged
merged 3 commits into from
Dec 14, 2019
Merged

add notebook_dir as trait to be deprecated #36

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4abc6a
add notebook_dir as trait to be deprecated
Zsailer Nov 30, 2018
a6f976b
check that notebook_dir and root_dir are the same
Zsailer Nov 30, 2018
9107c4e
deprecate notebook_dir following deprecation pattern from notebook
Zsailer Dec 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions jupyter_server/serverapp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,20 @@ def _update_pylab(self, change):
)
self.exit(1)

notebook_dir = Unicode(config=True,
help=_("DEPRECATED, use root_dir.")
)

@default('notebook_dir')
def _default_notebook_dir(self):
self.log.warning(_("\n notebook_dir is deprecated, use root_dir.\n"))
return self.root_dir

@validate('notebook_dir')
def _update_notebook_dir(self, change):
self.log.warning(_("\n notebook_dir is deprecated, use root_dir.\n"))
self.root_dir = change['value']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • With this behavior, there may be an issue when setting root_dir, and reading notebook_dir.

  • Maybe an observer on root_dir setting the notebook_dir trait through set_trait (bypassing validation) would be better.

  • However, we should be careful about incompatible setting of one vs the other?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, that should be an observe decorator.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. If root_dir changes, notebook_dir doesn't change.

I think setting notebook_dir in the root_dir observer should fix that. Why bypass validation? They should end up having the same validation logic right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding a

if self.notebook_dir != change['new']:
    self.notebook_dir = change['new']

to the root_dir observer, and

if self.root_dir != change['new']:
    self.root_dir = change['new']

to the notebook_dir observer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the traits are equal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to do some experiments. But it is not trivial.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

totally agree it's not trivial.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the check above if you want to test it. Just checks notebook_dir and root_dir are equal. If not, update the other trait.


root_dir = Unicode(config=True,
help=_("The directory to use for notebooks and kernels.")
)
Expand Down