-
Notifications
You must be signed in to change notification settings - Fork 85
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.
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
Jupyter notebook 5 compatibility #46
Conversation
54a2f32
to
7817ff4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TimShawver I updated this to drop support for IPython 3.x. I had two small suggestions on the other changes. Otherwise LGTM.
# support backing up the deleted directory in the OS trash can. | ||
# FileContentsManager should allow non-empty directories to be deleted. | ||
def test_delete_non_empty_dir(self): | ||
if issubclass(self.notebook.contents_manager_class, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably use isinstance(self.notebook.contents_manager, ...)
for these checks rather than issubclass
.
pgcontents/pgmanager.py
Outdated
klass = PostgresContentsManager | ||
kw = super(klass, self)._checkpoints_kwargs_default() | ||
except AttributeError: | ||
kw = {'parent': self, 'log': self.log} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was renamed to _default_checkpoints_kwargs
in notebook 5, so we can probably just use that instead of hard-coding a set of defaults.
…cluding when HybridContentsManager contains a FileContentsManager), and continue to test that non-empty folders owned by PostgresContentsManager are not deletable.
…version of IPython beyond checking that it's greater than or equal to 3.
- Drop references to IPython. - Add link to JupyterCon presentation.
…on 5.x (ipykernel gets installed with the notebook). Adds missing comma in ipycompat.py, some comment cleanup.
…ed (I think). Rename the notebook 4 and 5 installations to reflect the fact that ipython is no longer a dependency.
e7c1072
to
c21aef4
Compare
Adds on to #44 to make the changes suggested by this comment: https://github.com/quantopian/pgcontents/pull/44/files#r192937051