Make isort repeatable (nasa#694)

* make isort repeatable

* isort now consistently groups astrobee imports as first party; one-time update to apply to old files

.isort.cfg

@@ -0,0 +1,18 @@
+[settings]
+
+# The src_paths setting should be a comma-separated list of all the
+# folders in the astrobee repo that contain *.py files. Python files
+# found in these folders count as "first party" so they should be in a
+# separate group from the "third party" imports. If src_paths is not
+# specified, isort will treat only the files it's asked to check as
+# first party. Therefore, if we specify src_paths broadly in this way,
+# isort behavior should be more repeatable between (1) manually
+# running isort on a single file in your local dev machine, vs. (2)
+# running the git pre-commit hook locally on your dev machine, which
+# historically ran isort only on the files that changed since the last
+# commit, or (3) running the CI workflow, which always runs isort on
+# all files. If src_paths needs to be updated, like if *.py files are
+# added to a new folder, you can auto-update it by running
+# scripts/git/configure_isort_paths.sh.
+
+src_paths = doc/scripts,hardware/eps_driver/tools,hardware/pico_driver/scripts,hardware/pmc_actuator/tools,localization/localization_common,localization/localization_common/scripts/localization_common,localization/marker_tracking/tools/marker_tracking_node,localization/sparse_mapping/scripts,localization/sparse_mapping/tools,scripts/build,scripts/calibrate,scripts/debug,scripts/git,scripts/postprocessing/coverage_analysis,tools/bag_processing/scripts,tools/bag_processing/scripts/utilities,tools/bag_processing/test,tools/calibration/scripts,tools/gds_helper/src,tools/gnc_visualizer/dds,tools/gnc_visualizer/scripts,tools/gnc_visualizer/scripts/communications,tools/localization_analysis/scripts,tools/performance_tester/scripts

hardware/pico_driver/scripts/debug_pico_utils.py

@@ -31,11 +31,12 @@
 import logging
 
 import numpy as np
-import pico_utils as pico
 import rosbag
 from matplotlib import collections as mc
 from matplotlib import pyplot as plt
 
+import pico_utils as pico
+
 
 def plot_xy_grid_many(inbag_path, verbose=False, fast=False, cam=pico.DEFAULT_CAM):
     """

hardware/pico_driver/scripts/pico_check_split_extended.py

@@ -29,6 +29,7 @@
 import logging
 
 import numpy as np
+
 import pico_utils as pico
 
 

hardware/pico_driver/scripts/pico_split_extended.py

@@ -28,9 +28,10 @@
 import shutil
 
 import numpy as np
-import pico_utils as pico
 import rosbag
 
+import pico_utils as pico
+
 
 class PrintEveryK:
     def __init__(self, k):

hardware/pico_driver/scripts/pico_write_xyz_coeff.py

@@ -26,6 +26,7 @@
 import logging
 
 import numpy as np
+
 import pico_utils as pico
 
 

scripts/calibrate/extrinsics_calibrate.py

@@ -27,9 +27,10 @@
 import numpy as np
 import numpy.linalg
 import yaml
-from calibration_utils import *
 from tf import transformations
 
+from calibration_utils import *
+
 
 # returns extrinsics (T_cam_imu) from the yaml file
 def read_yaml_extrinsics(filename, cameras):

scripts/calibrate/intrinsics_calibrate.py

@@ -27,9 +27,10 @@
 import numpy as np
 import numpy.linalg
 import yaml
-from calibration_utils import *
 from tf import transformations
 
+from calibration_utils import *
+
 
 # returns intrinsics, distortion, and transforms between cameras from a yaml file
 def read_yaml(filename, cameras):

scripts/git/configure_isort_paths.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+
+# Updates the src_paths setting in the .isort.cfg file at the top
+# level. See that file for more details.
+
+thisdir=$(dirname "$0")
+srcdir=$(cd $thisdir/../.. && pwd)
+
+cd $srcdir
+
+# Generate a comma-separated list of folders containing *.py files
+pydirs=$(find . -name "*.py" -print0 | xargs -0 dirname | cut -c3- | sort | uniq | paste -sd "," -)
+
+# Overwrite the src_paths line in the config file to use the list
+perl -i -ple "if (/^src_paths = /) { \$_ = 'src_paths = $pydirs'; }" .isort.cfg

scripts/git/pre-commit.linter_python

@@ -17,10 +17,10 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-files=$(git diff --diff-filter=d --cached --name-only | grep '\.py$')
+py_changed=$(git diff --diff-filter=d --cached --name-only | grep '\.py$')
 
-# If files is empty exit success
-if [ -z "${files}" ]; then
+# If py_changed is empty exit success
+if [ -z "${py_changed}" ]; then
   echo "=================================================="
   echo "  No Python files changed, no checks needed."
   exit 0
@@ -46,21 +46,36 @@ fi
 echo "=================================================="
 echo "  Analysing python code style with 'black'."
 # This check the files but they will not be commited
-if `black . --include ${files} --check --quiet`; then
+if `black . --include ${py_changed} --check --quiet`; then
     echo "Linter checks using 'black' passed."
 else
     echo "Errors detected with 'black'. Fixing them. Try to add and commit your files again."
-    black . --include ${files}
+    black . --include ${py_changed}
     failed_lint=true
 fi
 
 echo "=================================================="
 echo "  Analysing python code style with 'isort'."
-if $(isort ${files} --extend-skip cmake --profile black --diff --check-only --quiet >/dev/null); then
+
+# We're running isort recursively on the top-level folder (not
+# limiting it to the changed files) in order to match how it is
+# invoked in the CI workflow. This is because isort determines what
+# imports are treated as first party, and therefore should be grouped
+# separately, in part based on what files it is asked to process. We
+# definitely don't want to have any disagreement where this pre-commit
+# hook wants things grouped one way but the CI workflow says that's
+# wrong. Note that our intention is for isort to treat all imports
+# found in the astrobee repo as first party, and we specify that using
+# the src_paths setting in the .isort.cfg file.  But we should still
+# run isort consistently in both places to avoid disagreement, which
+# could happen for example if the .isort.cfg src_paths list gets out
+# of date.
+
+if $(isort . --extend-skip cmake --profile black --diff --check-only --quiet >/dev/null); then
     echo "Linter checks using 'isort' passed."
 else
     echo "Errors detected with 'isort'. Fixing them. Try to add and commit your files again."
-  isort ${files} --extend-skip cmake --profile black >/dev/null
+  isort . --extend-skip cmake --profile black >/dev/null
   failed_lint=true
 fi
 

scripts/postprocessing/coverage_analysis/coverage_analyzer.py

@@ -11,10 +11,11 @@
 import sys
 import timeit
 
-import constants
 import numpy as np
 from tf.transformations import *
 
+import constants
+
 
 class Coverage_Analyzer:
     def __init__(self):

tools/bag_processing/scripts/apply_histogram_equalization_to_images.py

@@ -27,10 +27,11 @@
 import cv2
 import rosbag
 import rospy
-import utilities.utilities
 from cv_bridge import CvBridge, CvBridgeError
 from sensor_msgs.msg import Image
 
+import utilities.utilities
+
 if __name__ == "__main__":
     parser = argparse.ArgumentParser(
         description=__doc__, formatter_class=argparse.ArgumentDefaultsHelpFormatter

tools/bag_processing/scripts/check_bag_for_gaps.py

@@ -25,6 +25,7 @@
 import sys
 
 import rosbag
+
 import utilities.utilities
 
 if __name__ == "__main__":

tools/bag_processing/scripts/rosbag_debayer.py

@@ -27,10 +27,11 @@
 import cv2
 import rosbag
 import rospy
-import utilities.utilities
 from cv_bridge import CvBridge, CvBridgeError
 from sensor_msgs.msg import Image
 
+import utilities.utilities
+
 
 def convert_bayer(
     bagfile,

tools/bag_processing/scripts/rosbag_sample.py

@@ -33,9 +33,10 @@
 
 import genpy
 import rosbag
-import rosbag_rewrite_types as rrt
 import roslib
 
+import rosbag_rewrite_types as rrt
+
 
 def sample_bags(inbag_paths, outbag_path, rules_files, verbose=False):
     if os.path.exists(outbag_path):

tools/bag_processing/scripts/rosbag_splice.py

@@ -34,10 +34,11 @@
 import cv2
 import rosbag
 import rospy
-import utilities.utilities
 from cv_bridge import CvBridge, CvBridgeError
 from sensor_msgs.msg import Image
 
+import utilities.utilities
+
 
 def print_info(splice_timestamps, bag_start_time, bag_end_time):
     print(str(len(splice_timestamps)) + " splice timestamps selected.")

tools/bag_processing/scripts/rosbag_trim.py

@@ -24,6 +24,7 @@
 import sys
 
 import rosbag
+
 import utilities.utilities
 
 

tools/calibration/scripts/save_images_with_target_detections.py

@@ -28,11 +28,12 @@
 
 import aslam_cv_backend as acvb
 import cv2
-import get_bags_with_topic
 import kalibr_camera_calibration as kcc
 import kalibr_common as kc
 import numpy as np
 
+import get_bags_with_topic
+
 
 class Corner:
     def __init__(self, corner_id, target_corner, image_corner):

tools/gnc_visualizer/scripts/communications/dds_support.py

@@ -21,6 +21,8 @@
 from sys import path as sysPath
 from time import sleep
 
+from pkg_resources import get_distribution, parse_version
+
 from data_support import (
     ControlState,
     EkfState,
@@ -31,7 +33,6 @@
     Quaternion,
     Vector3,
 )
-from pkg_resources import get_distribution, parse_version
 
 try:
     import rticonnextdds_connector as rti

tools/localization_analysis/scripts/bag_and_parameter_sweep.py

@@ -26,9 +26,10 @@
 import shutil
 import sys
 
+import pandas as pd
+
 import bag_sweep
 import localization_common.utilities as lu
-import pandas as pd
 import parameter_sweep
 import plot_parameter_sweep_results
 

tools/localization_analysis/scripts/depth_odometry_parameter_sweep.py

@@ -32,9 +32,10 @@
 import multiprocessing
 import os
 
+import numpy as np
+
 import config_creator
 import localization_common.utilities as lu
-import numpy as np
 import parameter_sweep_utilities
 import plot_parameter_sweep_results
 

tools/localization_analysis/scripts/imu_analyzer.py

@@ -27,8 +27,9 @@
 import os
 import sys
 
-import imu_measurements
 import matplotlib
+
+import imu_measurements
 import plot_helpers
 
 matplotlib.use("pdf")

tools/localization_analysis/scripts/parameter_sweep.py

@@ -32,10 +32,11 @@
 import multiprocessing
 import os
 
+import numpy as np
+
 import average_results
 import config_creator
 import localization_common.utilities as lu
-import numpy as np
 import parameter_sweep_utilities
 import plot_parameter_sweep_results
 

tools/localization_analysis/scripts/plot_helpers.py

@@ -18,6 +18,7 @@
 # under the License.
 
 import matplotlib
+
 import poses
 import vector3ds
 

tools/localization_analysis/scripts/plot_results.py

@@ -26,8 +26,9 @@
 import os
 import sys
 
-import loc_states
 import matplotlib
+
+import loc_states
 import plot_helpers
 import poses
 import rmse_utilities

tools/localization_analysis/scripts/poses.py

@@ -18,10 +18,11 @@
 # under the License.
 
 import numpy as np
+import scipy.spatial.transform
+
 import orientations
 import pose
 import pose_covariances
-import scipy.spatial.transform
 import vector3ds
 
 

tools/localization_analysis/scripts/rmse_utilities.py

@@ -21,9 +21,10 @@
 import math
 
 import numpy as np
-import poses
 import scipy.spatial.transform
 
+import poses
+
 
 # Assumes poses_a and poses_b are sorted in time
 def get_same_timestamp_poses(

tools/localization_analysis/scripts/test_rmse_utilities.py

@@ -21,6 +21,7 @@
 import unittest
 
 import numpy as np
+
 import poses
 import rmse_utilities
 

tools/localization_analysis/scripts/utilities.py

@@ -16,6 +16,7 @@
 # under the License.
 
 import numpy as np
+
 import pose
 import poses
 

tools/localization_analysis/scripts/vector3d_plotter.py

@@ -18,6 +18,7 @@
 # under the License.
 
 import matplotlib
+
 import poses
 import vector3ds
 

tools/localization_analysis/scripts/vector3ds.py

@@ -18,6 +18,7 @@
 # under the License.
 
 import numpy as np
+
 import vector3d