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

Rework presentation of keywords #393

Merged
merged 61 commits into from
Aug 11, 2024
Merged

Conversation

kellertuer
Copy link
Member

@kellertuer kellertuer commented Jun 7, 2024

This is heavily WIP (just had a bit on time on the train with not so much concentration).

This aims to

  • find a format for keyword arguments that is better readable in both the rendered docs and REPL
  • -> for not I have a newline too much in the HTML format (hence rendering it here might be nice)
  • indicate defaults for keywords is for now done with an emoji in the last line (a bit too much offset for now).

🛣️ Roadmap

  • find a better way of presenting keyword arguments
  • start collecting common keywords and their defaults in a central “string collection” to reduce duplication in strings (and hence avoid possible multiple typos / indifferences when updating parts)
  • rework solver interfaces
    • plans/
    • solvers/
      • ARC and its sub solvers, especially Lanczos
      • ALM
      • Chambolle-Pock
      • CG
      • CBM
      • CPPA
      • DoC
      • DCPPA
      • DR
      • EPM
      • Frank_Wolfe_method and its state
      • gradient_descent and its state
      • LM
      • NM
      • PSO
      • PDRSNA
      • PBM
      • QN
      • TCG
      • SGM
      • Stochastic Gradient
      • TR
      • conjugate residual (and its plan)
      • interior point newton (and its plan)
  • generally
    • work through replaces `:(\s+)\(`(.*)`\) to =$2`: (maybe with a newline)
    • similarly for `:(\s+)\([`(.*)`\)` (keyword with linking examples
    • Introduce Documenterinterlinks for the functions (after most doc strings are no longer raw)
      • default_retraction_method
      • default_inverse_retraction_method
      • zero_vector
      • rand
      • at_iteration field should be unified
    • unify use of indices
      • k for iterations
      • I for inequality constraints g_i
      • j for equality constraints h_j
    • carefully check to add more links e.g. for
      • get_gradient
      • get_cost
      • get_iterate
    • extend the Nelder-Mead documentation and write down the whole algorithm in the doc string.
    • check and fix why it seems that :WhenActive is broken in https://manoptjl.org/stable/tutorials/HowToDebug/#Subsolver-debug

@kellertuer kellertuer added documentation WIP Work in Progress (for a pull request) labels Jun 7, 2024
Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 99.71014% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.76%. Comparing base (489da96) to head (65d6381).

Files Patch % Lines
src/solvers/adaptive_regularization_with_cubics.jl 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   99.78%   99.76%   -0.02%     
==========================================
  Files          76       77       +1     
  Lines        8226     8256      +30     
==========================================
+ Hits         8208     8237      +29     
- Misses         18       19       +1     

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

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@kellertuer
Copy link
Member Author

I think I found a good format by now and with the snippets we also avoid “stringduplication” – that is for example the “first parameter is a manifold” text will soon be the same in all doc strings.

@kellertuer kellertuer changed the title Experiment with keyword formats Rework presentation of keywords Jun 20, 2024
@kellertuer
Copy link
Member Author

kellertuer commented Jun 20, 2024

I think I have converged to a nice new format, and with a bit of rewriting how the doc string is defined, this allows to reuse parts as well – for example prominent keywords like evaluation=

Format

* `keyword=default`:
  description

is nice, since the = also indicates that what follows is the default.
All doc strings will be reformulated as “non-raw” to allow for interpolation. this means raw parts, like formulae, should be defined before.

Now that this is clear, this PR is really just a large work to check all doc strings and reform them. But the benefit is, that this reduces a lot of duplication in the strings as well, since common tens are now defined in plans/docstring_snippets.jl
both lines – the keyword=default and the description are possible strings to be collected,
whenever used more than once or twice.

Still a bit of work to do, where I will try to work on a doc string every day.

@kellertuer kellertuer marked this pull request as ready for review June 22, 2024 10:22
@kellertuer kellertuer removed the WIP Work in Progress (for a pull request) label Aug 10, 2024
@kellertuer
Copy link
Member Author

So now this should be good to go. Not sure it is worth its own version, since I will not activate vale (basically text style check) for now, since they have too often breaking changes and though I implemented Julia code stuff for them the newest version reports, .jl files are not allowed in checks.

@kellertuer kellertuer added the Ready-for-Review A label for pull requests that are feature-ready label Aug 10, 2024
@kellertuer
Copy link
Member Author

Since this is really just involving doc string rewrites, I will merge this now and include it in the next release (the other PR still open). It will hence already reside on master for a while then (without a new version).

@kellertuer kellertuer merged commit c8564b8 into master Aug 11, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Ready-for-Review A label for pull requests that are feature-ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant