small doc improvements from user testing

[nhoening]

Some comments which came from a user conversation today.

I also wonder about one more thing I cannot judge by myself: The horizon parameter is not considered optional any more for some API versions < v2 (under some circumstances), but the docstring still list it under **Optional fields** (v1/routes.py::post_meter_data). Even the validator is called optional_horizon_accepted but I'm not sure if that paints the real picture by now or could be more clear.

[Flix6x]

Right now, for API versions <2, the horizon field is only optional for servers in play mode (set to 0 hours by default), but perhaps we can use the same default for all modes of operation, by removing line 415 in validators.py.

[nhoening]

So your suggestion is to remove line 415 in validators.py, which makes the field not optional anymore, and then also adjust the documentation / docstrings I mentioned?

[Flix6x]

No, removing line 415 actually makes the horizon field completely optional again (a missing horizon will be turned into a 0 horizon). As far as I can tell at the moment, only lines 380 and 381 in the corresponding docstring would need to be adjusted. It's worth an API changelog entry, too, though. It kind of rolls back the 3rd breaking change under v2.0 | 2021-04-02. I write "kind of" because the default inference (for non-play servers) used to be to set the belief time equal to the server time (leading to different horizons for each belief), whereas now (when deleting line 415) the default inference is to set the belief horizon to zero (leading to the same horizon for each belief).

Need to take another look, please hold

[Flix6x]

I hope it makes more sense now, and I'm glad to see the horizon is now optional again for all API versions.

For future reference, we ran into this because I failed to avoid that 3rd breaking change from #41.

[Flix6x]

Wait, no. Describing this as a breaking change for v2.0 would be wrong for two reasons:

• It should not impact v2.0, but versions <2.0 only. Version 2 already inferred a prior (for non-play servers) or a horizon (for play servers) instead.
• It is not a breaking change, but partially reverting a breaking change by reinstantiating the inference, but changing the inference policy.
  I pushed a commit that should fix this issue.

My remaining concern is about structuring the API changelog itself. The entry is listed under v2, but the change only impacts lower versions. I don't have a good answer on how to restructure, but I can explain how I handled this before.

The entries are not listed chronologically, but grouped by the most recent API version they impact, with some earlier entries also impacting multiple versions (which is noted with a comment).

I suggest we move the 3rd breaking change of v2.0-2 to a new entry v1.3-8 (with the same date as v2.0-2 and a comment *Affects all versions since v1.0*.), and to move the current entry v2.0-3 to a new entry v1.3-9, also with a comment *Affects all versions since v1.0*.

[nhoening]

"I pushed a commit that should fix this issue." Which one do you mean?

[Flix6x]

Apparently I forgot to actually push my commit. I did just now, after resolving merge conflicts in the changelog.

[Flix6x]

I approve, but now you should probably review my changes in c5923ae.

[nhoening]

So maybe now is the time to document what we mean with this sentence in the configuration documentation if FLEXMEASURES_MODE: "This is used to turn on certain extra behaviours." There are two modes our code recognizes, demo and play.

• Demo: ability to display login credentials, ability to keep data from one year, but keep FlexMeasures functioning with all other years as well. Both of these are controlled with other config settings. Anything else?
• Play: Can you fill in what happens in play mode?

[nhoening]

@Flix6x The use of current_app "outside" the function like this is not possible, as there is no application context at that point. I learned that from all our tests falling, as importing this file breaks.

We have to find a different way. If, for play mode, infer_missing is always True for horizons ad False for prior, maybe it could go into the respective validators.

[Flix6x]

Thanks for finding out. I'll move the logic back into the validators, and devise of some other way of letting v1.x and v2 endpoints choose to use this logic. Alas, infer_missing is not always True for horizons in play mode. Specifically, it is False for the postPrognosis endpoint v1.x, even in play mode. I could make the boolean optional, and set the default to None implying inference, so endpoints can be explicit or let the validator choose a default depending on server mode.

For the horizon validator:

• v1.x postPrognosis: infer_missing=False
• v1.x postOther: infer_missing=True
• v2.x postPrognosis: infer_missing=False
• v2.x postOther: infer_missing=None -> True if in play, False otherwise

For the prior validator:

• v1.x doesn't use this validator
• v2.x postPrognosis and postOther: infer_missing=None -> False if in play, True otherwise

So to be clear, the differences between v1.x and v2 are that:

• The only time a user needs to be bothered with communicating the timing of their beliefs is for the postPrognosis endpoint: for v1.x this is in all modes, whereas for v2, this is in play mode only.
• The inference policy for v1.x is beliefs formed right after the fact, whereas for v2, this is beliefs formed when communicated to FM.

[Flix6x]

So maybe now is the time to document what we mean with this sentence in the configuration documentation if FLEXMEASURES_MODE: "This is used to turn on certain extra behaviours." There are two modes our code recognizes, demo and play.

• Demo: ability to display login credentials, ability to keep data from one year, but keep FlexMeasures functioning with all other years as well. Both of these are controlled with other config settings. Anything else?

Yes, a lot. I did this exercise for play mode just now, and compiled the list below. I suggest you make a separate issue for this, because it looked like there had been even more customisation for the demo mode.

• Play: Can you fill in what happens in play mode?

Play

Use this mode to turn on several FlexMeasures features offering better support for simulations.

Big features

• [Data] Allows overwriting existing data when saving data to the database.

• [API] The inferred recording time of incoming data is immediately after the event took place rather than the actual time at which the server received the data.

• [API] Posting price or weather data does not trigger forecasting jobs.

• [API] Registers the restoreData endpoint enabling database resets through the API.

• [API] Automatically creates a new weather sensor when posting weather data for a new location, instead of returning the nearest available weather sensor to post data to.

Small features

• [API] Posted UDI events are not enforced to be consecutive.

• [API] Names in GetConnectionResponse are the connections' unique database names rather than their display names (this feature is planned to be deprecated).

• [UI] The dashboard plot showing the latest power value is not enforced to lie in the past (in case of simulating future values).

• [Documentation] The restoreData endpoint is left out of the changelog. [I don't think this works anymore in the new public non-server-bound documentation].

[nhoening]

Thanks! I can cover demo mode.
I believe it would make a nice addition to this random-docs-improvements PR.

[nhoening]

I believe how that explicit decorator parameter could become clearer is by calling it "infer_X_override_modes" and it receives a list. By default the list is empty so we don't need it always. And we could expand it by other modes.

[Flix6x]

We discussed adjusting the decorator parameter infer_missing_play to something that overrides the infer_missing parameter, for play or some list of server modes, by instructing the validator to do the opposite of whatever the infer_missing parameter tells it to do. After some more consideration I would like to vote against such renaming, with the following arguments:

• Letting the interpretation of one parameter depend on another makes setting up the decorator more prone to error.
• I plan to move the optional_horizon_accepted and optional_prior_accepted to a polyfield marshmallow decorator, including replacement of the mode dependent inference policy with a dedicated config variable.

[nhoening]

As I said earlier, I won't force my opinion through here. Let's get this thing out the door.