Skip to content

Resolve "UpdateParameters has to be called manually from the python API"

Victor Poughon requested to merge 1842-update-parameter into develop

Summary

  • Automatically call UpdateParameters() in the python API in all SetParameterXXX() methods. It is still available to the Python API, but there should be no need to call it manually.
  • Add a test to make sure the original error reported in #1842 (closed) does not occur.
  • Update documentation of "UpdateParameters corner case".
  • Fix missing python documentation for parameters:

For example the generated example documentation of TrainImagesClassifier currently looks like this:

1842-before

With this branch it looks like:

1842-after

One caveat with this implementation is that the order in which parameters are set is now important, and out-of-order parameter setting from the Python API can now cause an error in some applications DoUpdate logic. Note that this is already the case for the GUI interface. It is not currently the case for the CLI interface because the order in which parameters are set is not the order in which they are given on the command line. If the above tradeoff is not acceptable to you, feel free to mention it in this review.

Rationale

Closes #1842 (closed).

There was quite some discussion in #1842 (closed) on how to solve this issue. There were alternative proposals to fix it including (among others):

  • Removing UpdateParameters completely
  • Moving checks from ListView::SetValue to ListView::GetValue
  • Adding a fine-grained parameter dependency system to the application engine

The proposal in this MR is the only one I managed to complete and it actually fixes the issue, and I think it is also the simplest. It's not ideal but I think it's better than the current state of the code in develop regarding usage of the python API and its documentation. If there are other alternative proposals we can always revert this and take a look at them.

Copyright

The copyright owner is CNES and has signed the ORFEO ToolBox Contributor License Agreement.

Check before merging:

  • All discussions are resolved
  • At least 2 👍 votes from core developers, no 👎 vote.
  • The feature branch is (reasonably) up-to-date with the base branch
  • Dashboard is green
  • Copyright owner has signed the ORFEO ToolBox Contributor License Agreement
  • Optionally, run git diff develop... -U0 --no-color | clang-format-diff.py -p1 -i on latest changes and commit

Merge request reports