refactor: failing actions (#474)

* add missing dicstrings * make docstring a bit longer * fix all flake8 errors * Add documentation on status checks * Update README.md --------- Co-authored-by: Mustafa Abdulrahman <[email protected]> Co-authored-by: Richard Abrich <[email protected]>
OpenAdaptAI · Aug 28, 2023 · 09f4e71 · 09f4e71
1 parent 2089e74
commit 09f4e71
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 48 deletions.
diff --git a/README.md b/README.md
@@ -264,7 +264,7 @@ The following pre-commit hooks are used in OpenAdapt:
 - [check-yaml](https://github.com/pre-commit/pre-commit-hooks#check-yaml): Validates the syntax and structure of YAML files.
 - [end-of-file-fixer](https://github.com/pre-commit/pre-commit-hooks#end-of-file-fixer): Ensures that files end with a newline character.
 - [trailing-whitespace](https://github.com/pre-commit/pre-commit-hooks#trailing-whitespace): Detects and removes trailing whitespace at the end of lines.
-- [black](https://github.com/psf/black): Formats Python code to adhere to the Black code style.
+- [black](https://github.com/psf/black): Formats Python code to adhere to the Black code style. Notably, the `--preview` feature is used.
 - [isort](https://github.com/PyCQA/isort): Sorts Python import statements in a consistent and standardized manner.
 
 To set up the pre-commit hooks, follow these steps:
@@ -279,6 +279,14 @@ pre-commit install
 
 Now, the pre-commit hooks are installed and will run automatically before each commit. They will enforce code quality standards and prevent committing code that doesn't pass the defined checks.
 
+### Status Checks
+
+When you submit a PR, the "Python CI" workflow is triggered for code consistency. It follows organized steps to review your code:
+
+1. **Python Black Check** : This step verifies code formatting using Python Black style, with the `--preview`` flag for style.
+
+2. **Flake8 Review** : Next, Flake8 tool thoroughly checks code structure, including flake8-annotations and flake8-docstrings. Though GitHub Actions automates checks, it's wise to locally run `flake8 .`` before finalizing changes for quicker issue spotting and resolution.
+
 # Submitting an Issue
 
 Please submit any issues to https://github.com/OpenAdaptAI/OpenAdapt/issues with the

diff --git a/openadapt/productivity.py b/openadapt/productivity.py
@@ -1,6 +1,6 @@
-"""
-This module generates an HTML page with information about the productivity of the user in the
-latest recording.
+"""This module generates an HTML page.
+
+The page has information about the productivity of the user in the latest recording.
 
 Usage:
 
@@ -64,10 +64,9 @@
 
 
 def find_gaps(action_events: list[ActionEvent]) -> Tuple[int, float]:
-    """
-    Find and count gaps between ActionEvents that are longer than MAX_GAP_SECONDS.
+    """Find and count gaps between ActionEvents that are longer than MAX_GAP_SECONDS.
 
-    Parameters:
+    Args:
         action_events (list[ActionEvent]): A list of ActionEvent objects.
 
     Returns:
@@ -87,10 +86,9 @@ def find_gaps(action_events: list[ActionEvent]) -> Tuple[int, float]:
 
 
 def find_clicks(action_events: list[ActionEvent]) -> int:
-    """
-    Count the number of mouse clicks in a list of ActionEvents.
+    """Count the number of mouse clicks in a list of ActionEvents.
 
-    Parameters:
+    Args:
         action_events (list[ActionEvent]): A list of ActionEvent objects.
 
     Returns:
@@ -104,10 +102,9 @@ def find_clicks(action_events: list[ActionEvent]) -> int:
 
 
 def find_key_presses(action_events: list[ActionEvent]) -> int:
-    """
-    Count the number of key presses in a list of ActionEvents.
+    """Count the number of key presses in a list of ActionEvents.
 
-    Parameters:
+    Args:
         action_events (list[ActionEvent]): A list of ActionEvent objects.
 
     Returns:
@@ -121,18 +118,17 @@ def find_key_presses(action_events: list[ActionEvent]) -> int:
 
 
 def is_within_margin(event1: ActionEvent, event2: ActionEvent, margin: int) -> bool:
-    """
-    Check if two mouse events are within a specified pixel distance from each other.
+    """Check if two mouse events are within a specified pixel distance from each other.
 
-    Parameters:
+    Args:
         event1 (ActionEvent): The first ActionEvent.
         event2 (ActionEvent): The second ActionEvent.
         margin (int): The maximum allowable distance in pixels between the mouse
         coordinates of the two events for them to be considered the same event.
 
     Returns:
-        bool: True if the distance between the mouse coordinates of the events is within the
-        specified margin, False otherwise.
+        bool: True if the distance between the mouse coordinates
+        of the events is within the specified margin, False otherwise.
     """
     return (
         abs(event1.mouse_x - event2.mouse_x) <= margin
@@ -141,12 +137,14 @@ def is_within_margin(event1: ActionEvent, event2: ActionEvent, margin: int) -> b
 
 
 def compare_events(event1: ActionEvent, event2: ActionEvent) -> bool:
-    """
-    Compare two action events to determine if they are similar enough to be considered
-    the same. For mouse events, clicks must be a within some distance of each other. For
+    """Compare two action events.
+
+    To determine if they are similar enough to be considered the same.
+
+    For mouse events, clicks must be a within some distance of each other. For
     key presses, the keys must be the same.
 
-    Parameters:
+    Args:
         event1 (ActionEvent): The first ActionEvent object to be compared.
         event2 (ActionEvent): The second ActionEvent object to be compared.
 
@@ -168,17 +166,20 @@ def find_num_tasks(
     length: int,
     task: Optional[list[ActionEvent]] = None,
 ) -> Tuple[list[ActionEvent], int, float]:
-    """
-    Given a list of ActionEvents, the start of a repeating task, the length of the task, and
-    optionally the identified task, verify that the task repeats (and if not,
-    find the correct repeating task), find how many times the task is repeated,
+    """Find the num of times a task is repeated and how much time is spent on the task.
+
+    Given a list of ActionEvents, the start of a repeating task,
+    the length of the task, and optionally the identified task,
+    verify that the task repeats (and if not, find the correct repeating task),
+    find how many times the task is repeated,
     and how much time in total is spent repeating the task.
 
-    Parameters:
+    Args:
         action_events (List[ActionEvent]): A list of ActionEvents.
         start (ActionEvent): The starting ActionEvent of the task.
         length (int): The number of ActionEvents in the identified task.
-        task (Optional[ActionEvent]): An optional task identified by the search algorithm.
+        task (Optional[ActionEvent]):
+        An optional task identified by the search algorithm.
 
     Returns:
         list[ActionEvent]: The final verified task.
@@ -267,11 +268,13 @@ def find_num_tasks(
 def rec_lrs(
     action_events: list[ActionEvent],
 ) -> Tuple[list[ActionEvent], Optional[ActionEvent], int]:
-    """
+    """A function to find the longest repeating non-overlapping task of ActionEvents.
+
     Caller function that calls longest_repeated_substring recursively to find the
-    longest repeating non-overlapping task of ActionEvents from the original action_events.
+    longest repeating non-overlapping task of ActionEvents
+    from the original action_events.
 
-    Parameters:
+    Args:
         action_events (List[ActionEvent]): A list of ActionEvents.
 
     Returns:
@@ -296,12 +299,14 @@ def rec_lrs(
 def longest_repeated_substring(
     action_events: list[ActionEvent],
 ) -> Tuple[list[ActionEvent], Optional[ActionEvent], int]:
-    """
+    """A function to find the longest repeating non-overlapping task of ActionEvents.
+
     Recursive function to find the longest repeating non-overlapping task of
     ActionEvents from the original action_events. Based on algorithm found at
-    this link: https://www.geeksforgeeks.org/longest-repeating-and-non-overlapping-substring/
+    this link:
+    https://www.geeksforgeeks.org/longest-repeating-and-non-overlapping-substring/
 
-    Parameters:
+    Args:
         action_events (List[ActionEvent]): A list of ActionEvents.
 
     Returns:
@@ -350,10 +355,9 @@ def longest_repeated_substring(
 
 
 def filter_move_release(action_events: list[ActionEvent]) -> list[ActionEvent]:
-    """
-    Filter out any events that aren't clicks and key presses.
+    """Filter out any events that aren't clicks and key presses.
 
-    Parameters:
+    Args:
         action_events (list[ActionEvent]): list of ActionEvents to be filtered
 
     Returns:
@@ -372,9 +376,14 @@ def filter_move_release(action_events: list[ActionEvent]) -> list[ActionEvent]:
     return filtered_action_events
 
 
-def find_errors(action_events: list[ActionEvent]):
-    """
-    Currently unused as there is no good way to find errors.
+def find_errors(action_events: list[ActionEvent]) -> int:
+    """Currently unused as there is no good way to find errors.
+
+    Args:
+        action_events (list[ActionEvent]): list of ActionEvents.
+
+    Returns:
+        int: number of errors.
     """
     # TODO: how to find click errors
     errors = 0
@@ -394,10 +403,9 @@ def find_errors(action_events: list[ActionEvent]):
 
 
 def find_num_window_tab_changes(window_events: list[WindowEvent]) -> int:
-    """
-    Find the number of times a user switches between tabs or applications.
+    """Find the number of times a user switches between tabs or applications.
 
-    Parameters:
+    Args:
         window_events (list[WindowEvent]): list of WindowEvents.
 
     Return:
@@ -420,10 +428,18 @@ def find_num_window_tab_changes(window_events: list[WindowEvent]) -> int:
     return num_window_tab_changes - 1
 
 
-def calculate_productivity():
-    """
-    Calculate any relevant information about the productivity of a user in the latest recording.
+def calculate_productivity() -> None:
+    """A function to calculate productivity metrics.
+
+    Calculate any relevant information
+    about the productivity of a user in the latest recording.
     Display this information in an HTML page and open the page.
+
+    Args:
+        None
+
+    Returns:
+        None
     """
     configure_logging(logger, LOG_LEVEL)
 
@@ -640,13 +656,13 @@ def calculate_productivity():
     logger.info(f"{fname_out=}")
     output_file(fname_out, title=title)
 
-    result = show(
+    result = show(  # noqa: F841
         layout(
             rows,
         )
     )
 
-    def cleanup():
+    def cleanup() -> None:
         os.remove(fname_out)
         removed = not os.path.exists(fname_out)
         logger.info(f"{removed=}")