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.

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

Also allow subclasses of TextFileContentsManager #218

Merged
merged 1 commit into from
Apr 16, 2019
Merged

Also allow subclasses of TextFileContentsManager #218

merged 1 commit into from
Apr 16, 2019

Conversation

153957
Copy link
Contributor

@153957 153957 commented Apr 16, 2019

Only load the jupytext.TextFileContentsManager as contents manager
if the configured class is not already an instance of it.

We use a subclass of TextFileContentsManager to extend the rename and delete methods. However, our subclass is replaced by the provided TextFileContentsManager since it is not exactly that class.
This allows custom classes if they are a subclass of TextFileContentsManager.

Only load the jupytext.TextFileContentsManager as contents manager
if the configured class is not already an instance of it.
@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

Merging #218 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #218   +/-   ##
=======================================
  Coverage   99.23%   99.23%           
=======================================
  Files          65       65           
  Lines        6142     6142           
=======================================
  Hits         6095     6095           
  Misses         47       47
Impacted Files Coverage Δ
jupytext/__init__.py 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 939d437...08b8951. Read the comment docs.

@mwouts
Copy link
Owner

mwouts commented Apr 16, 2019

This pull request introduces 1 alert when merging 08b8951 into 939d437 - view on LGTM.com

new alerts:

  • 1 for Wrong name for an argument in a class instantiation

Comment posted by LGTM.com

@153957
Copy link
Contributor Author

153957 commented Apr 16, 2019

The alert is not caused by these changes.

@mwouts mwouts added this to the 1.1.1 milestone Apr 16, 2019
mwouts added a commit that referenced this pull request Apr 16, 2019
@mwouts mwouts mentioned this pull request Apr 16, 2019
@mwouts mwouts merged commit 08b8951 into mwouts:master Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants