Skip to content

Move the jupytext-config helpers to a subpackage, and add tests#1167

Merged
mwouts merged 4 commits intomainfrom
coverage_jupytext_config
Nov 27, 2023
Merged

Move the jupytext-config helpers to a subpackage, and add tests#1167
mwouts merged 4 commits intomainfrom
coverage_jupytext_config

Conversation

@mwouts
Copy link
Copy Markdown
Owner

@mwouts mwouts commented Nov 23, 2023

Closes #1097

@mwouts
Copy link
Copy Markdown
Owner Author

mwouts commented Nov 23, 2023

@parmentelat does this look fine to you? Thanks!

@mwouts mwouts mentioned this pull request Nov 23, 2023
Copy link
Copy Markdown
Contributor

@parmentelat parmentelat left a comment

Choose a reason for hiding this comment

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

looks motly good to me except for the way we can configure the actual config file

Comment thread src/jupytext_config/labconfig.py Outdated
self.config = {}
# the state before any changes
self._prior_config = {}
self.settings_file = Path(settings_path) / "default_setting_overrides.json"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

imho if we're to be able to provide the location for the settings, i'td be better if the caller provides the whole path to the config file including its basename
my reason for that is, the configuration scheme within jupyterlab is truly complex, and with for example the overrides.d scheme, we may someday need to use another filename than this hard-coded default_setting_overrides.json

which amounts to dealing only with a single settings_path all over the place

@mwouts mwouts added this to the 1.16.0 milestone Nov 25, 2023
@mwouts mwouts force-pushed the coverage_jupytext_config branch from ed53fd9 to 1feace3 Compare November 25, 2023 17:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c37d9c2) 97.59% compared to head (15452de) 97.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1167      +/-   ##
==========================================
+ Coverage   97.59%   97.61%   +0.02%     
==========================================
  Files          26       29       +3     
  Lines        4357     4451      +94     
==========================================
+ Hits         4252     4345      +93     
- Misses        105      106       +1     
Flag Coverage Δ
external 75.23% <ø> (+0.11%) ⬆️
functional 88.54% <ø> (ø)
integration 77.37% <96.22%> (+0.53%) ⬆️
unit 66.64% <91.11%> (+0.25%) ⬆️

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.

Comment thread src/jupytext_config/labconfig.py Outdated
@mwouts mwouts force-pushed the coverage_jupytext_config branch from 839b83d to 154b876 Compare November 25, 2023 20:59
@mwouts mwouts requested a review from parmentelat November 25, 2023 22:20
@mwouts mwouts force-pushed the coverage_jupytext_config branch from 154b876 to 15452de Compare November 27, 2023 20:28
@mwouts mwouts merged commit 405617f into main Nov 27, 2023
@mwouts mwouts deleted the coverage_jupytext_config branch November 27, 2023 20:48
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.

Restore the coverage

3 participants