55 rewrite interface to enable non ai plugins #58
Conversation
This file contains 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
added 2 commits
October 12, 2023 11:13
…an architecure discussion still
Evana13G
force-pushed
the
55-rewrite-interface-to-enable-nonAI-plugins
branch
from
b5a6a50
to
f5dc9cc
Compare
…ion between regular AI plugins and complex reasoning plug ins
…epresenations, complex plugins
Some isolation issues exist with new function parse_plugins_list
parse_plugins_list is untested still
has errors
config_filepath does not exist in this function
is still called from other tests, but does not cause failures
Config files
changes reflect the location and name changes
using __init__.py files to locate modules
Example Plugin modules removed _plugin from directory/module name
<module_name>_plugin.py contains the Plugin Class for that module
Full unit test coverage on _plugin.py files and plugin_import
Note: plugin_import test contains a new testing concept
mocking __import__ builtin method intereferes with test fail output
mocker.stop is used to remove mocking before most test results
the __import__ checks remain but are done first before stop
after stop, test error output is intact, prior to stop INTERNALERROR
will occur and indicates problems with the __import__ only
this provides for testing with minimal output disruption
Added all arg plugins Added fake headers and tests as separate fakes Assert now in order of operations
Plugin lists are now dicts
term list was used to describe objects that are dicts
removed list and replaced with dict
ExecutionEngine
Moved RUN_FLAGS access into try block
lack of RUN_FLAGS in config raises KeyError
this now catches that and reports full config info like DEFAULT
still labeled as optional because missing flags result in False
Commented Plugins as required data
allowed to be empty dict
keywords must still be in config
KeyError no longer possible in parse_plugins_dict (formerly _list)
removed try block in function
Updated error messages in parse_plugins_dict for clarity
test_execution_engine
Updated tests for changes and full coverage+
the-other-james
requested changes
Oct 19, 2023
There was a problem hiding this comment.
Overall this looks good, and I'm really liking the direction we are going!
The big thing that this merge needs is an update to the README which includes a diagram that shows:
- How the four plugin types fit in
- The flow of low level and high level
- How vehicle representation fits into this
Maybe that is big enough to be its own issue/merge... but we need something to help newcomers understand the system.
|
I'm also happy to make a diagram with mermaid if you send me a sketch. |
Unit tests and full coverage of learners_interface.py Removed newline in learners_interface.py
Codecov Report
@@ Coverage Diff @@
## main #58 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 24 +2
Lines 942 986 +44
Branches 131 134 +3
=========================================
+ Hits 942 986 +44
|
test_salient_event_does_nothing in learners_interface missing LearnersInterface now test_LearnersInterface_salient_event_does_nothing test_planners_interface.py added missing test section comments removed unnecessary imports
asgibson
reviewed
Oct 20, 2023
…esis construct updating
I sent you the link to the original one, but I wont be able to update with the new interfaces until next week |
update function
test was randomly missing coverage with a 0 length argument
led to discovery that updates were not passing correct data
vehicle data now updated first with frame
now a separate function
then tests are run and status updated with results
then constructs updated with curr_data
now in its own function
unit tests updated for changes and full coverage
Agent reason function was not properly sending curr_data to learning systems now updating data and using curr_data no more sending arguments to render_resoning (was not used in func)
Simplified empty subsystems test Added test for all empty labels Simplified success test used index 'i' for label keys to ensure unique keys reduced arrange phase setup
cfirth-nasa
approved these changes
Oct 25, 2023
the-other-james
approved these changes
Oct 25, 2023
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.
No description provided.