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

Global section not being picked up (or imprecise docs) #521

Open
krassowski opened this issue Apr 15, 2020 · 5 comments
Open

Global section not being picked up (or imprecise docs) #521

krassowski opened this issue Apr 15, 2020 · 5 comments

Comments

@krassowski
Copy link
Member

If I set my nbdime_config.json to:

{
  "Global": {
    "Ignore": {
      "/cells/*/execution_count": false
    },
    "details": false
  }
}

I expect the details and Ignore to be applied to every nbdime tool, given that

Global Options to apply to all commands.
sections

However, it is not applied to anything:

nbdime --config output fragment:

NbDiff:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

full output in details

All available config options, and their current values:

NbDiff:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

NbDiffWeb:
  Ignore: {}
  attachments: <unset, resolves to true>
  base_url: "/"
  browser: null
  color_words: false
  details: <unset, resolves to true>
  ip: "127.0.0.1"
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  persist: false
  port: 0
  sources: <unset, resolves to true>
  workdirectory: "/home/krassowski/xx"

NbMerge:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  ignore_transients: true
  input_strategy: null
  merge_strategy: "inline"
  metadata: <unset, resolves to true>
  output_strategy: null
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

NbMergeWeb:
  Ignore: {}
  attachments: <unset, resolves to true>
  base_url: "/"
  browser: null
  color_words: false
  details: <unset, resolves to true>
  ignore_transients: true
  input_strategy: null
  ip: "127.0.0.1"
  merge_strategy: "inline"
  metadata: <unset, resolves to true>
  output_strategy: null
  outputs: <unset, resolves to true>
  persist: false
  port: 0
  sources: <unset, resolves to true>
  workdirectory: "/home/krassowski/xx"

NbShow:
  Ignore: {}
  attachments: <unset, resolves to true>
  details: <unset, resolves to true>
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

Server:
  base_url: "/"
  browser: null
  ip: "127.0.0.1"
  persist: false
  port: 8888
  workdirectory: "/home/krassowski/xx"

Extension:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

NbDiffDriver:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

NbDiffTool:
  Ignore: {}
  attachments: <unset, resolves to true>
  base_url: "/"
  browser: null
  color_words: false
  details: <unset, resolves to true>
  ip: "127.0.0.1"
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  persist: false
  port: 0
  sources: <unset, resolves to true>
  workdirectory: "/home/krassowski/xx"

NbMergeDriver:
  Ignore: {}
  attachments: <unset, resolves to true>
  color_words: false
  details: <unset, resolves to true>
  ignore_transients: true
  input_strategy: null
  merge_strategy: "inline"
  metadata: <unset, resolves to true>
  output_strategy: null
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

NbMergeTool:
  Ignore: {}
  attachments: <unset, resolves to true>
  base_url: "/"
  browser: null
  color_words: false
  details: <unset, resolves to true>
  ignore_transients: true
  input_strategy: null
  ip: "127.0.0.1"
  merge_strategy: "inline"
  metadata: <unset, resolves to true>
  output_strategy: null
  outputs: <unset, resolves to true>
  persist: false
  port: 0
  sources: <unset, resolves to true>
  workdirectory: "/home/krassowski/x"

Using Diff instead works:

{
  "Diff": {
    "Ignore": {
      "/cells/*/execution_count": false
    },
    "details": false
  }
}

nbdime --config output fragment:

NbDiff:
  Ignore:
    /cells/*/execution_count: false
  attachments: <unset, resolves to true>
  color_words: false
  details: false
  metadata: <unset, resolves to true>
  outputs: <unset, resolves to true>
  sources: <unset, resolves to true>

full output in details

``` All available config options, and their current values:

NbDiff:
Ignore:
/cells/*/execution_count: false
attachments: <unset, resolves to true>
color_words: false
details: false
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

NbDiffWeb:
Ignore:
/cells/*/execution_count: false
attachments: <unset, resolves to true>
base_url: "/"
browser: null
color_words: false
details: false
ip: "127.0.0.1"
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
persist: false
port: 0
sources: <unset, resolves to true>
workdirectory: "/home/krassowski/xx"

NbMerge:
Ignore: {}
attachments: <unset, resolves to true>
color_words: false
details: <unset, resolves to true>
ignore_transients: true
input_strategy: null
merge_strategy: "inline"
metadata: <unset, resolves to true>
output_strategy: null
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

NbMergeWeb:
Ignore: {}
attachments: <unset, resolves to true>
base_url: "/"
browser: null
color_words: false
details: <unset, resolves to true>
ignore_transients: true
input_strategy: null
ip: "127.0.0.1"
merge_strategy: "inline"
metadata: <unset, resolves to true>
output_strategy: null
outputs: <unset, resolves to true>
persist: false
port: 0
sources: <unset, resolves to true>
workdirectory: "/home/krassowski/xx"

NbShow:
Ignore: {}
attachments: <unset, resolves to true>
details: <unset, resolves to true>
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

Server:
base_url: "/"
browser: null
ip: "127.0.0.1"
persist: false
port: 8888
workdirectory: "/home/krassowski/xx"

Extension:
Ignore:
/cells/*/execution_count: false
attachments: <unset, resolves to true>
color_words: false
details: false
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

NbDiffDriver:
Ignore:
/cells/*/execution_count: false
attachments: <unset, resolves to true>
color_words: false
details: false
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

NbDiffTool:
Ignore:
/cells/*/execution_count: false
attachments: <unset, resolves to true>
base_url: "/"
browser: null
color_words: false
details: false
ip: "127.0.0.1"
metadata: <unset, resolves to true>
outputs: <unset, resolves to true>
persist: false
port: 0
sources: <unset, resolves to true>
workdirectory: "/home/krassowski/xx"

NbMergeDriver:
Ignore: {}
attachments: <unset, resolves to true>
color_words: false
details: <unset, resolves to true>
ignore_transients: true
input_strategy: null
merge_strategy: "inline"
metadata: <unset, resolves to true>
output_strategy: null
outputs: <unset, resolves to true>
sources: <unset, resolves to true>

NbMergeTool:
Ignore: {}
attachments: <unset, resolves to true>
base_url: "/"
browser: null
color_words: false
details: <unset, resolves to true>
ignore_transients: true
input_strategy: null
ip: "127.0.0.1"
merge_strategy: "inline"
metadata: <unset, resolves to true>
output_strategy: null
outputs: <unset, resolves to true>
persist: false
port: 0
sources: <unset, resolves to true>
workdirectory: "/home/krassowski/xx"

</details>
@krassowski
Copy link
Member Author

krassowski commented Apr 15, 2020

$ nbdime --version
2.0.0
$ python --version
Python 3.8.1
$ jupyter --version
jupyter core     : 4.6.1
jupyter-notebook : 6.0.3
qtconsole        : 4.6.0
ipython          : 7.11.1
ipykernel        : 5.1.3
jupyter client   : 5.3.4
jupyter lab      : 2.1.0
nbconvert        : 5.6.1
ipywidgets       : 7.5.1
nbformat         : 5.0.3
traitlets        : 4.3.3

@krassowski
Copy link
Member Author

Are there any tests for configuration?

@vidartf
Copy link
Collaborator

vidartf commented Apr 29, 2020

Ah, I think this is a case of poorly written documentation on my part. The global config object is for all commands, but only for options that are supported by all commands, which is currently only log_level:

nbdime/nbdime/config.py

Lines 104 to 110 in 8804407

class Global(NbdimeConfigurable):
log_level = Enum(
('DEBUG', 'INFO', 'WARN', 'ERROR', 'CRITICAL'),
'INFO',
help="Set the log level by name.",
).tag(config=True)

I can see it being sub-optimal that the _Ignorables class is currently kept private, as it means there is no common key to use to configure diff, merge and show ignorables in one category.

Possible actionables:

  1. Update the documentation to better reflect the current state of things.
  2. Expose the Ignorables config target (possibly with a clearer name?).
  3. Refactor the config system to allow any config value on any target ? (I will not have time to do this work).
  4. And possibly improve the CLI tools to better inspect the current config (ideas/PRs welcome).

@vidartf
Copy link
Collaborator

vidartf commented Sep 29, 2020

I think I have a fix for this, but since the config resolution changes, I'll do it as a major version upgrade together with the upgrade to lab3 (I'll do a minor release before that for the other PRs recently merged).

@vidartf
Copy link
Collaborator

vidartf commented Sep 29, 2020

(You can test it in this branch until then if you want)

brunetton added a commit to brunetton/nbdime that referenced this issue May 5, 2021
brunetton added a commit to brunetton/nbdime that referenced this issue May 5, 2021
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

No branches or pull requests

2 participants