fix(events): say which event types could not be interpreted - #67
Merged
Conversation
get_events dropped event types this orlab has no enum member for and told the caller only through a log line. The returned dict then looked complete: a caller iterating it could not tell "this simulation had no such event" from "orlab did not recognise the event and dropped it", and an application that raises its log level saw nothing at all. It matters most on exactly the jars #61 concerned. A newer OpenRocket running on a nearest-older profile is where unknown event types appear, so the case where rows go missing is the case where profile_exact is already False. get_events now returns EventTimes, carrying unknown_event_types. It is a dict subclass, so callers that index or compare the result are unaffected, and the log line stays -- but the log can be silenced and the field cannot. Closes #63
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_eventsdropped event types this orlab has no enum member for and told the calleronly through a log line (
helper.py:195-203). The returned dict then lookedcomplete: a caller iterating it could not tell "this simulation had no such event"
from "orlab did not recognise the event and dropped it", and an application that raises
its log level saw nothing at all.
It matters most on exactly the jars #61 concerned. A newer OpenRocket running on a
nearest-older profile is where unknown event types appear -- so the case where rows go
missing from a result you are about to analyse is the same case where
profile_exactis already
False.Shape
get_eventsnow returnsEventTimes, adictsubclass carryingunknown_event_types: frozenset[str].The issue asked that "I could not interpret part of this result" be readable from the
return value, not only from a log. A
Helperattribute would have been the otheroption and matches #61's
profile_exact, but it is last-call state on an object reusedacross simulations, so it would be wrong under
SimulationPool. Subclassingdictkeeps it per-result and costs existing callers nothing -- indexing,
==against a plaindict, and
dict(events)all behave as before, which is asserted.The log line stays. The point is that the log can be silenced and the field cannot.
Tests
Four added to
tests/test_events.py, using the existing no-JVM fakes. Three go red withthe source reverted and the tests kept; the fourth is the backward-compatibility control
and passes either way, which is what makes it a control. One of the three silences the
logger to
CRITICALand asserts the drop is still visible -- that is the precise failuremode.
test_api_surface.pyupdated, sinceEventTimesis public API.just checkandjust testgreen: 235 passed, 2 skipped (baseline 231).Docs
docs/guides/multi-version.mddescribed the old behaviour ("skipped byget_eventswith a logged warning") and now shows the field.
Closes #63