Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

24 allow for dynamic plugins #36

Merged
merged 15 commits into from
Sep 14, 2023
Merged

24 allow for dynamic plugins #36

merged 15 commits into from
Sep 14, 2023

Conversation

cfirth-nasa
Copy link
Contributor

Add support for external plugins to be specified by name and path in config INI's and imported as Plugins in data_driven_learning.py

@cfirth-nasa cfirth-nasa linked an issue Sep 11, 2023 that may be closed by this pull request
@cfirth-nasa cfirth-nasa added the enhancement New feature or request label Sep 11, 2023
@cfirth-nasa cfirth-nasa added this to the OnAIR 1.0 refactor milestone Sep 11, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #36 (e5cfee4) into main (7372f2a) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #36   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        24    -1     
  Lines         1169      1104   -65     
  Branches       156       144   -12     
=========================================
- Hits          1169      1104   -65     
Files Changed Coverage Δ
...a_driven_components/ai_plugin_abstract/__init__.py 100.00% <ø> (ø)
.../data_driven_components/ai_plugin_abstract/core.py 100.00% <ø> (ø)
...src/data_driven_components/data_driven_learning.py 100.00% <100.00%> (ø)
onair/src/reasoning/agent.py 100.00% <100.00%> (ø)
onair/src/run_scripts/execution_engine.py 100.00% <100.00%> (ø)
onair/src/run_scripts/sim.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@asgibson asgibson force-pushed the 24-allow-for-dynamic-plugins branch from e5cfee4 to f4006d2 Compare September 11, 2023 19:09
Copy link
Contributor

@Evana13G Evana13G left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small tweaks

@@ -6,6 +6,7 @@ TelemetryFiles = ['700_crash_to_earth_1.csv']
ParserFileName = csv_parser
ParserName = CSV
SimName = CSV
PluginList = {'generic_plugin':'plugins/generic/generic_plugin.py'} # format: {plugin_name : plugin_path}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an area where we can put an assertion statement?

importlib.import_module('onair.src.data_driven_components.' + plugin_name + '.' + f'{plugin_name}_plugin').Plugin(plugin_name, headers) for plugin_name in _ai_plugins
]
self.ai_constructs = []

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertion here, or earlier. Also, I'd remove this empty space.

@@ -15,9 +15,9 @@
from ..reasoning.diagnosis import Diagnosis

class Agent:
def __init__(self, vehicle):
def __init__(self, plugin_list, vehicle):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd swap the order (most significant to least significant)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, follows the instantiation order

@@ -94,6 +98,8 @@ def parse_configs(self, config_filepath):
self.benchmarkIndices = config['DEFAULT']['BenchmarkIndices']
except:
pass
## Sort Data: Plugins
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is sorting happening? I think this function turns a string into list (with extra processing of tokens , beyond pythons built in "list()").

@@ -39,7 +39,7 @@ def __init__(self, simType, parsedData, SBN_Flag):

else:
self.simData = DataSource(parsedData.get_sim_data())
self.agent = Agent(vehicle)
self.agent = Agent(plugin_list,vehicle)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swap

@Evana13G
Copy link
Contributor

@cfirth-nasa Nice job :D I ran a pedantic review, happy to discuss if there are questions. I'll merge after these small tweaks. Note, some of the code changes will have ramifications in the test files. Thank youuuu!

@cfirth-nasa
Copy link
Contributor Author

Will have time to work on this tomorrow - functionality is there but unit tests need work. Leaving up in case someone (Alan) can get to it before I can

DataDrivenLearning is now guaranteed to receive valid paths
Parsing Plugins is now with required parsing
Using ast to check and ensure integrity of dict from config
  ValueError when not a dict or empty dict
  FileNotFoundError when path for Plugin is invalid
@Evana13G Evana13G merged commit 0500300 into main Sep 14, 2023
2 checks passed
@asgibson asgibson deleted the 24-allow-for-dynamic-plugins branch September 18, 2023 18:00
the-other-james pushed a commit to dennisafa/OnAIR that referenced this pull request Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow for dynamic plugins
4 participants