Feature/agent checking

[SeanScripts]

Creates AgentCheckerUtil class, with functions for finding reachable/unreachable pages and intents. Some other checking functions could be added in the future. Also adds a function to TestCases to get a dataframe of test case results for an agent, rerunning tests without results, or optionally rerunning all test cases. I had put this in the agent checking class, but decided it made more sense to include it in TestCases directly, since we have similar functions to get dataframes in the other core classes.

[kmaphoenix]

@MRyderOC please review PR and make optimization suggestions, particularly around the following:

• Method length (Should aim for ~40 lines per style guide)
• Nested statements (can these be condensed? Refactored?)
• Arg Inputs (can these be condensed?)

[MRyderOC]

Some general points:

• Most of the functions don't have proper documentation. Please follow Google Python Style Guide: Comments and Docstrings and make the changes accordingly.
• To condense the input parameters, use either id or name for page and flow and be consistent with your choice. e.g. only use name (flow_name, page_name). If you want the user to have the ability to pass either name or id, define an internal helper method to identify which one is passed as an argument and return the one we're using in our code. e.g.

def helper_identify_flow_name_or_id(self, flow_name_or_id):
    # identify whether "flow_name_or_id" is a "flow_name" or a "flow_id"
    # Assuming that we use "flow_name" as an argument for methods like 'find_reachable_pages'
    if flow_name_or_id is like flow_name:
        return flow_name_or_id
    else:
        return self.flow_map[flow_name_or_id]

• Most of the if statements could be more brief. I addressed some of them as "Brief if statement" to explain how we can achieve the same result with less code. I noticed that most of them use the hasattr built-in function.

• Note that hasattr(obj, name) returns True if obj has the name attribute even though the value of the obj.name is None and for all the proto objects that we're working with the result of hasattr will be True.

• For verbose=True change the print statements to logging.info and provide more readable information for the user.

[MRyderOC]

1. Could you please provide more information on intent_route_limit and is_initial?
2. It seems include_start_page_routes only affects the flow's Start Page (Line 519). That means if we pass some random page other than the Start page to from_page and pass include_start_page_routes it will return all reachable pages from both from_page and Start Page.

[SeanScripts]

1. I added some documentation about intent_route_limit. is_initial should have been internal here. I realized that I broke the functionality earlier by trying to put is_initial into the params dict, but I think I managed to fix it.

2. Yes, that's the intended behavior. If include_start_page_routes is true, then start page intent routes will be treated as if they are in scope on every page, which is the way they actually work in the agent. This isn't very useful if you're starting from a page other than the start page with no limit on the number of intent routes taken. But with intent_route_limit = 1 for example, you can answer the question: "Starting from from_page, which pages can be reached in one conversation turn? (assuming all conditional routes are possible)". Choosing whether or not to allow start page routes to be in scope is pretty useful here.

[MRyderOC]

@kmaphoenix Would you please take a quick look at the comments? I'll appreciate your input.

[kmaphoenix]

@SeanScripts one way to optimize this section would be to create dictionary of your page_id mappings.

Then you can call .get on the dictionary directly and return INVALID if nothing matches.
This should significantly cut down on the number of if/else checks you need to do.

page_dict = {
  'END_SESSION': 'End Session',
  'END_FLOW': 'End Flow',
  etc.
}

display_name = page_dict.get(page_id, 'INVALID')

[SeanScripts]

I added this dictionary and returned the mapping through it if the page ID was one of these special cases. However, we still need to look up the page name from the main dictionary of page names if it's not one of these special cases. I simplified the part after this a little bit by using .get.

Also, I know the latest version of Scrapi added the special cases to the list of pages, but I'm not sure what the format is, compared to what is returned in the test cases. In particular, whether or not the "pages" like END_SESSION include the flow ID. I might need to check that.

[SeanScripts]

Looks like with the 1.6 update to get_pages_map, this check for the special pages isn't needed at all. Thanks! This function has been simplified.

[kmaphoenix]

@SeanScripts If you have a commented section in a method where you are describing what that section does, you can just make that entire section a new method.

This will clean up your main method and provide better debug tracking later.

[SeanScripts]

I moved this section to a new function and had it return a single-row dataframe for each test case response, which avoided initializing all the empty lists above. I kept the check for test cases that needed to be retested since it was just one condition.

[SeanScripts]

@kmaphoenix I wrote some tests, though I wasn't able to run them using pytest in my environment. However, running the content of the tests with a demo agent, they're still working as expected.

[kmaphoenix]

Linted and tested locally.