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

Decomplexify/document result-rendering switches #6174

Merged
merged 3 commits into from
Nov 11, 2021

Conversation

mih
Copy link
Member

@mih mih commented Nov 10, 2021

The full story is in #6038

Concretely, this:

  • documents the 'disabled' renderer mode
  • strip supported for the undocumented datalad.api.result-renderer
    config setting
  • homogeneous documentation for --output-format and result_renderer
  • rename default[_result_renderer] to generic[_result_renderer to
    resolve the multi-level ambiguity of "default" for a renderer that,
    ironically, is not the default. Shims for legacy code are in place,
    and will likely never be removed
  • replace internal usage of output_format with a consistent
    result_renderer for homogeneity. Command line API is kept unchanged
    though -- might be worth a follow-up discussion in a separate issue/PR
  • discontinue the undocumented use of result_renderer=None and replaces
    it with result_renderer='disabled'

Fixes #6038

mih added 2 commits November 10, 2021 16:48
The full story is in datalad#6038

Concretely, this:

- documents the 'disabled' renderer mode
- strip supported for the undocumented datalad.api.result-renderer
  config setting
- homogeneous documentation for `--output-format` and `result_renderer`
- rename `default[_result_renderer]` to `generic[_result_renderer` to
  resolve the multi-level ambiguity of "default" for a renderer that,
  ironically, is not the default. Shims for legacy code are in place,
  and will likely never be removed
- replace internal usage of `output_format` with a consistent
  `result_renderer` for homogeneity. Command line API is kept unchanged
  though -- might be worth a follow-up discussion in a separate issue/PR

Fixes datalad#6038
Which exploited a side-effect of the specific implementation.
The intended, and now documented usage is result_renderer=`disabled`.
@mih mih added the semver-minor Increment the minor version when merged label Nov 10, 2021
@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #6174 (0c0fbe5) into master (981aa95) will decrease coverage by 47.17%.
The diff coverage is 49.19%.

❗ Current head 0c0fbe5 differs from pull request most recent head f059d85. Consider uploading reports for the commit f059d85 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #6174       +/-   ##
===========================================
- Coverage   89.70%   42.52%   -47.18%     
===========================================
  Files         318      317        -1     
  Lines       41896    41617      -279     
===========================================
- Hits        37584    17699    -19885     
- Misses       4312    23918    +19606     
Impacted Files Coverage Δ
datalad/core/distributed/tests/test_push.py 0.00% <0.00%> (-98.94%) ⬇️
datalad/core/local/tests/test_create.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/core/local/tests/test_diff.py 0.00% <0.00%> (-99.54%) ⬇️
datalad/core/local/tests/test_results.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/core/local/tests/test_run.py 16.71% <ø> (-83.29%) ⬇️
datalad/core/local/tests/test_status.py 0.00% <0.00%> (-98.48%) ⬇️
datalad/customremotes/datalad.py 0.00% <0.00%> (-35.94%) ⬇️
datalad/distributed/create_sibling_ria.py 15.38% <0.00%> (-66.49%) ⬇️
...ad/distributed/tests/test_create_sibling_ghlike.py 0.00% <0.00%> (-69.70%) ⬇️
...talad/distributed/tests/test_create_sibling_ria.py 0.00% <0.00%> (-100.00%) ⬇️
... and 289 more

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 981aa95...f059d85. Read the comment docs.

@mih
Copy link
Member Author

mih commented Nov 11, 2021

This is ready. Will merge EOB today, unless objections are expressed.

Copy link
Member

@bpoldrack bpoldrack left a comment

Choose a reason for hiding this comment

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

No objections. :)

@mih mih merged commit 9a5b066 into datalad:master Nov 11, 2021
@mih mih deleted the defaultrenderer branch November 11, 2021 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Describe result_renderer kwarg semantics
2 participants