Check ContentsManager base class using inspect

[mahendrapaipuri]

Closes #1239

@mwouts What do you think of this change? I have bumped jupytext version in tests to use new SyncMetaManager

[github-actions]

Thank you for making this pull request.

Did you know? You can try it on Binder: or .

Also, the version of Jupytext developed in this PR can be installed with pip:

HATCH_BUILD_HOOKS_ENABLE=true pip install git+https://github.com/mahendrapaipuri/jupytext.git@check_contents_mgr_type

(this requires nodejs, see more at Developing Jupytext)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.04%. Comparing base (29a979f) to head (303c8fb).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1254      +/-   ##
==========================================
- Coverage   97.76%   97.04%   -0.72%     
==========================================
  Files          29       29              
  Lines        4468     4468              
==========================================
- Hits         4368     4336      -32     
- Misses        100      132      +32     

Flag	Coverage Δ
external	?
functional	?
integration	?
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mwouts]

Hi @mahendrapaipuri , thank you for the PR. Yes that sounds great! And it's exactly the test that I wanted, thanks.

Maybe we could also say a word about how to use Jupytext with jupyter-fs==1.0 in the documentation (I mean, the test is great but not everyone will find it).

I'm not sure what happened to the CI? In a recent run it was green but I have seen a few varying failures recently.

[mahendrapaipuri]

Maybe we could also say a word about how to use Jupytext with jupyter-fs==1.0 in the documentation (I mean, the test is great but not everyone will find it).

Yes, I agree. I forgot about the docs part. I will update the PR with a word about it.

I'm not sure what happened to the CI? In a recent run it was green but I have seen a few varying failures recently.

I think the failure is due to a bug in jupytext==1.0.0 that has been fixed recently. But there was no new release with this bugfix. Should we revert to jupytext==0.4.0 in the tests?

[mwouts]

Maybe we could also say a word about how to use Jupytext with jupyter-fs==1.0 in the documentation (I mean, the test is great but not everyone will find it).

Yes, I agree. I forgot about the docs part. I will update the PR with a word about it.

Thanks Mahendra. I can also take care of that over the week-end.

I think the failure is due to a bug in jupytext==1.0.0 that has been fixed recently. But there was no new release with this bugfix. Should we revert to jupytext==0.4.0 in the tests?

You mean in jupyter-fs? It looks like the CI fails on the lab extension test, and in the logs there is indeed something about jupyter-fs. Maybe installing jupyter-fs changes the default contents manager now?

[mahendrapaipuri]

You mean in jupyter-fs? It looks like the CI fails on the lab extension test, and in the logs there is indeed something about jupyter-fs. Maybe installing jupyter-fs changes the default contents manager now?

Yes, indeed I meant jupyter-fs. No, it is not about the contents manager. I think it is more due to an error in UI due to the bug in jupyter-fs that has been fixed. I will try to look into it in more detail

[mahendrapaipuri]

So, when we use jupyter-fs, we need to set ContentsManager class to the one shipped by jupyter-fs and if it is not configured, the extension will not be registered.

Moreoever, jupyter-fs is overriding the browser-test shipped by jupyter-lab to test its own functionality. As in the lab extension test in CI, we are not setting ContentsManager to the one from the jupyter-fs, the extension is not being activated and hence browser test fails.

I guess we should not "test" functionality of jupyter-fs extension in jupytext CI. I propose that we move lab extension test as the last step in workflow and uninstall jupyter-fs before lab extension test so that we wont have these issues.

@mwouts What do you think? Btw, there is a PR to bump jupyter-fs version with bug fixes. Maybe we can wait until they cut a new release?

[mwouts]

Hi Mahendra, thank you for looking into this!

I agree with your suggestion to uninstall jupyter-fs before running the UI tests.

Do we need the next jupyter-fs release to make the tests pass? If we don't I would rather take your PR sooner, as I am thinking of publishing a new release.

[mahendrapaipuri]

@mwouts No, we dont need to wait for the release for the tests to pass. I added a comment to keep track of it in pyproject.toml and pushed new changes. Let me know what do you think!

[mwouts]

Thank you @mahendrapaipuri ! That looks great. I will fix the issue with the CI, take your PR (I might relax the requirement on jupyterfs in tests from ==1.0.0 to >=1.0 if that's fine with you) and publish a new release asap.

[mahendrapaipuri]

I might relax the requirement on jupyterfs in tests from ==1.0.0 to >=1.0 if that's fine with you

That was a good idea. Cheers!