fix #514, call_data_callbacks after shared the data

[jmorice91]

!!!INSERT YOUR DESCRIPTION HERE!!!

List of things to check before making a PR

Before merging your code, please check the following:

• you have added a line describing your changes to the Changelog;
• you have added unit tests for any new or improved feature;
• In case you updated dependencies, you have checked pdi/docs/CheckList.md
• you have checked your code format:
  • you have checked that you respect all conventions specified in CONTRIBUTING.md;
  • you have checked that the indentation and formatting conforms to the .clang-format;
  • you have documented with doxygen any new or changed function / class;
• you have correctly updated the copyright headers:
  • your institution is in the copyright header of every file you (substantially) modified;
  • you have checked that the end-year of the copyright there is the current one;
• you have updated the AUTHORS file:
  • you have added yourself to the AUTHORS file;
  • if this is a new contribution, you have added it to the AUTHORS file;
• you have added everything to the user documentation:
  • any new CMake configuration option;
  • any change in the yaml config;
  • any change to the public or plugin API;
  • any other new or changed user-facing feature;
  • any change to the dependencies;
• you have correctly linked your MR to one or more issues:
  • your MR solves an identified issue;
  • your commit contain the Fix #issue keyword to autoclose the issue when merged.

[benoitmartin88]

Great feature to add. Thanks !
Not much is needed to merge this PR.
I think this feature would require a unit test. Could you add this?

[benoitmartin88]

Almost ready to merge.
I do have a few questions that need answering concerning error handling, potentially superfluous functions and writing tests in C++.

[benoitmartin88]

I think this is almost ready to merge.
Could you make sure the indent and spacing of the modified files is correct ?
Thanks !

[jmorice91]

I think this is almost ready to merge. Could you make sure the indent and spacing of the modified files is correct ? Thanks !

Is it normal that the action indent doesn't catch that?

[Yushan-Wang]

Ready to merge

[Yushan-Wang]

@jbigot Could you review this PR please?

[jbigot]

This is a great submission! My main comment is about moving Var_to_reclaim of the provider side instead of expecting each user to implement it. Overall this is really nice though!

[jbigot]

What's the status for this one?

[Yushan-Wang]

Looks good to me!
However, I have difficulties understanding the test.
Could you please add a test with hdf5, where the name of the file is a variable?
Before, we had to expose first the filename_size, then the filename. With your change, the order of the expose is then irrelevant.
Or exposing a vector who's size is also a variable?

[jbigot]

For Multiple_error, you could base your work on top of https://github.com/pdidev/pdi/tree/improved_errmsg
I'm waiting for the formatting & renaming of errors to be merged before I make a PR

[JAuriac]

Maybe shouldn't use a Global_context directly, and should remove the direct include to global_context.h, to go through Context instead with virtuals

[jmorice91]

Good catch. We must not use Global_context directly. This variables is not defined in the "pdi/include"
We have three options:

• 1. Use instead the class Context as you propose (implemented and tested but not commited yet).
• 2. Do a virtual class Delayed_data_callbacks and put the implementation in the src as Data_descriptor
     and use Global_context in implementation
• 3. Do a virtual class Delayed_data_callbacks and put the implementation in the src as Data_descriptor
     and use Context in the implementation

[JAuriac]

What is the "same behaviour" here ? We need this emplace_back to reproduce the behaviour where we overwrite the data definition that is already available ?

[jmorice91]

If you call this function,

PDI_multi_expose("my_test", "pdi_var1", &var1, PDI_OUT, "pdi_var1", &var1, PDI_OUT, NULL);

The callback on data "pdi_var1" is called two times in pdi v1.10.
For example, if the callback corresponds to the user_code function my_func, we call two times the function my_func.

[JAuriac]

Is writing over const variables intended here (technically ok because not constexpr) ?

[jmorice91]

Yes, indeed I need to remove the const here.