fixed review comments

observation_portal/common/configdb.py

@@ -453,7 +453,7 @@ def get_max_rois(self, instrument_type_code):
             if instrument_type_code.upper() == instrument['instrument_type']['code'].upper():
                 return instrument['science_cameras'][0]['camera_type']['max_rois']
 
-    def get_diagnol_ccd_fov(self, instrument_type_code, autoguider=False):
+    def get_diagonal_ccd_fov(self, instrument_type_code, autoguider=False):
         ''' Get the diagonal fov in arcminutes for the ccd, from the camera_type pscale and pixelsx/y in configdb
         '''
         for instrument in self.get_instruments():

observation_portal/common/test_data/configdb.json

@@ -166,7 +166,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBIG",
                       "name": "1M0-SCICAM-SBIG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -331,7 +330,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBIG",
                       "name": "1M0-SCICAM-SBIG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -496,7 +494,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBIG",
                       "name": "1M0-SCICAM-SBIG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -679,7 +676,6 @@
                     "camera_type": {
                       "code": "2M0-FLOYDS-SCICAM",
                       "name": "2M0-FLOYDS-SCICAM",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -880,7 +876,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBIG",
                       "name": "1M0-SCICAM-SBIG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -1056,7 +1051,6 @@
                     "camera_type": {
                       "code": "1M0-NRES-SCICAM",
                       "name": "1M0-NRES-SCICAM",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -1225,7 +1219,6 @@
                     "camera_type": {
                       "code": "2M0-FLOYDS-SCICAM",
                       "name": "2M0-FLOYDS-SCICAM",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -1437,7 +1430,6 @@
                     {
                       "code": "mc03g",
                       "camera_type": {
-                        "size": "26.4x26.4",
                         "pscale": 1.0,
                         "name": "2.0 meter Muscat G Camera",
                         "code": "2M0-PIXIS-G",
@@ -1472,7 +1464,6 @@
                     {
                       "code": "mc03r",
                       "camera_type": {
-                        "size": "26.4x26.4",
                         "pscale": 1.0,
                         "name": "2.0 meter Muscat R Camera",
                         "code": "2M0-PIXIS-R",
@@ -1507,7 +1498,6 @@
                     {
                       "code": "mc03i",
                       "camera_type": {
-                        "size": "26.4x26.4",
                         "pscale": 1.0,
                         "name": "2.0 meter Muscat I Camera",
                         "code": "2M0-PIXIS-I",
@@ -1542,7 +1532,6 @@
                     {
                       "code": "mc03z",
                       "camera_type": {
-                        "size": "26.4x26.4",
                         "pscale": 1.0,
                         "name": "2.0 meter Muscat Z Camera",
                         "code": "2M0-PIXIS-Z",
@@ -1578,7 +1567,6 @@
                   "autoguider_camera": {
                     "code": "mc03g",
                     "camera_type": {
-                      "size": "26.4x26.4",
                       "pscale": 1.0,
                       "name": "2.0 meter Muscat G Camera",
                       "code": "2M0-PIXIS-G",
@@ -1775,7 +1763,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBIG",
                       "name": "1M0-SCICAM-SBIG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -1940,7 +1927,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBXX",
                       "name": "1M0-SCICAM-SBXX",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -2126,7 +2112,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SBAG",
                       "name": "1M0-SCICAM-SBAG",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,
@@ -2311,7 +2296,6 @@
                     "camera_type": {
                       "code": "1M0-SCICAM-SOAR",
                       "name": "1M0-SCICAM-SOAR",
-                      "size": "3x4",
                       "pixels_x": 1000,
                       "pixels_y": 1000,
                       "pscale": 0.5,

observation_portal/requestgroups/dither.py

@@ -5,8 +5,8 @@ def expand_dither_pattern(dither_details):
     '''
     Takes in a valid configuration and valid set of dither parameters, and expands the instrument_configs within the
     configuration with offsets to fit the dither pattern details specified.
-    :param configuration_dict: a valid configuration dictionary.
-    :param dither_details: a valid dictionary of dither pattern parameters
+    :param dither_details: a valid dictionary containing the `configuration`, and a number of dither
+                           parameters such as `num_points`, `pattern`, `point_spacing` and others.
     :return: Configuration with expanded list of instrument_configurations.
     '''
     configuration_dict = dither_details.get('configuration', {})
@@ -17,11 +17,8 @@ def expand_dither_pattern(dither_details):
     if pattern == 'line':
         offsets = calc_line_offsets(dither_details.get('num_points'), dither_details.get('point_spacing'), dither_details.get('orientation'),
                                     dither_details.get('center'))
-    elif pattern == 'box':
-        offsets = calc_box_offsets(dither_details.get('point_spacing'), dither_details.get('line_spacing'),
-                                   dither_details.get('orientation'), sides_angle=270)
     elif pattern == 'spiral':
-        offsets = calc_spiral_offsets(dither_details.get('num_points'), dither_details.get('point_spacing'), dither_details.get('orientation'))
+        offsets = calc_spiral_offsets(dither_details.get('num_points'), dither_details.get('point_spacing'))
     elif pattern == 'grid':
         offsets = calc_grid_offsets(dither_details.get('num_rows'), dither_details.get('num_columns'), dither_details.get('point_spacing'),
                                     dither_details.get('line_spacing'), dither_details.get('orientation'), dither_details.get('center'))
@@ -62,32 +59,11 @@ def calc_line_offsets(num_points, point_spacing, orient, center):
     return offsets
 
 
-def normalize_angle(angle):
-    """Normalize an <angle> so that: 0 <= angle_norm < 2 * np.pi"""
-    twopi = 2.0 * pi
-
-    if angle >= twopi:
-        m = floor(angle/twopi)
-        if angle/twopi - m > 0.99999:   # account for rounding errors
-            m += 1
-        angle_norm = angle - m * twopi
-    elif angle < 0:
-        m = ceil(angle/twopi)
-        if angle/twopi - m < -0.99999:   # account for rounding errors
-            m -= 1
-        angle_norm = abs(angle - m * twopi)
-    else:
-        angle_norm = angle
-
-    return angle_norm
-
-
-def calc_spiral_offsets(num_points, point_spacing, orientation):
+def calc_spiral_offsets(num_points, point_spacing):
     """ Calculates offsets for a spiral pattern spaced <point_spacing> arcseconds apart and spiraling outward
-        from the origin until <num_points> is reached. The initial angle out is the orientation angle.
-        Points are calculated using this equation: https://math.stackexchange.com/questions/2335055/placing-points-equidistantly-along-an-archimedean-spiral-from-parametric-equatio
+        from the origin until <num_points> is reached. Points are calculated using this equation: 
+        https://math.stackexchange.com/questions/2335055/placing-points-equidistantly-along-an-archimedean-spiral-from-parametric-equatio
     """
-    # TODO: work orientation angle into this, right now it isn't used
     offsets = [(0,0),]
 
     n = 1
@@ -103,43 +79,6 @@ def calc_spiral_offsets(num_points, point_spacing, orientation):
     return offsets
 
 
-def calc_box_offsets(point_spacing, line_spacing, orient=0, sides_angle=-90):
-    """Calculates offsets for a BOX (4 point) dither pattern spaced
-    <point_spacing> arcseconds apart horizontally and <line_spacing> arcseconds
-    apart vertically orientated along  <orient> degrees East of North
-    (clockwise from North through East) with <sides_angle> clockwise from one
-    line segment to the next
-    Returns a list of tuples for the offsets"""
-    # TODO: work number of points into this? Not sure an arbitrary number of points makes sense if we assume
-    # That a box should be a closed loop and end at its origin.
-    offsets = [(0,0),]
-
-    # Offsets from origin
-    x0 = y0 = 0.0
-
-    num_points = 4 # fixed 4 point BOX for now
-
-    for angle,segment_num in zip([orient, orient+sides_angle+180, 360-(180-orient)], range(1, num_points)):
-
-        # Normalize accumulated angle to 0..2*pi (0..360deg)
-        norm_angle = normalize_angle(radians(angle))
-
-        # If its the 1st or 3rd line segment, use point_spacing as length
-        # otherwise use line_spacing
-        if (segment_num-1) % 2 == 0:
-            distance = point_spacing
-        else:
-            distance = line_spacing
-        # Angles measured clockwise from North ("y-axis") rather than anti-clockwise
-        # from East ("x-axis") so sin/cos flipped compared to normal
-        x_offset = (distance * sin(norm_angle)) + x0
-        y_offset = (distance * cos(norm_angle)) + y0
-        x0 = x_offset
-        y0 = y_offset
-        offsets.append((x_offset, y_offset))
-    return offsets
-
-
 def calc_grid_offsets(num_rows, num_columns, point_spacing, line_spacing, orient=90, center=False):
     """Calculates offsets for a grid of <num_points> (must be a square number
     if scalar or a tuple of (n_columns x n_rows) grid points) with <point_spacing>

observation_portal/requestgroups/serializers.py

@@ -851,7 +851,7 @@ def validate_requests(self, value):
 
 class DitherSerializer(serializers.Serializer):
     configuration = import_string(settings.SERIALIZERS['requestgroups']['Configuration'])()
-    pattern = serializers.ChoiceField(choices=('line', 'box', 'grid', 'spiral'), required=True)
+    pattern = serializers.ChoiceField(choices=('line', 'grid', 'spiral'), required=True)
     num_points = serializers.IntegerField(required=False)
     point_spacing = serializers.FloatField(required=True)
     line_spacing = serializers.FloatField(required=False)
@@ -864,18 +864,13 @@ def validate_configuration(self, configuration):
         if len(configuration.get('instrument_configs', [])) > 1:
             raise serializers.ValidationError(_("Cannot expand a configuration for dithering with more than one instrument_config set"))
 
-        target = configuration.get('target')
-        if target.get('type') == 'ICRS':
-            if abs(90-target.get('dec')) < 1 or abs(-90-target.get('dec')) < 1:
-                raise serializers.ValidationError(_('Target declination must be greater than 1 degree from the pole to support dither generation'))
-
         return configuration
 
     def validate(self, data):
         validated_data = super().validate(data)
         if 'num_points' not in validated_data and validated_data.get('pattern') in ['line', 'spiral']:
             raise serializers.ValidationError(_('Must specify num_points when selecting a line or spiral dither pattern'))
-        if 'line_spacing' not in validated_data and validated_data.get('pattern') in ['grid', 'box']:
+        if 'line_spacing' not in validated_data and validated_data.get('pattern') == 'grid':
             # Set a default line spacing equal to the point spacing if it is not specified
             validated_data['line_spacing'] = validated_data['point_spacing']
         if validated_data.get('pattern') == 'grid':

observation_portal/requestgroups/test/test_api.py

@@ -877,19 +877,6 @@ def test_expansion_fails_for_multiple_instrument_configs(self):
         self.assertEqual(response.status_code, 400)
         self.assertIn('Cannot expand a configuration for dithering with more than one instrument_config set', str(response.content))
 
-    def test_expansion_fails_if_target_dec_close_to_pole(self):
-        configuration = self.configuration.copy()
-        configuration['target']['dec'] = 89.5
-        dither_data = {
-            'configuration': configuration,
-            'num_points': 4,
-            'pattern': 'line',
-            'point_spacing': 2
-        }
-        response = self.client.post(reverse('api:configuration-dither'), data=dither_data)
-        self.assertEqual(response.status_code, 400)
-        self.assertIn('Target declination must be greater than 1 degree from the pole to support dither generation', str(response.content))
-
     def test_expansion_fails_if_numpoints_not_specified_for_line(self):
         configuration = self.configuration.copy()
         dither_data = {

observation_portal/requestgroups/views.py

@@ -128,8 +128,8 @@ def get(self, request):
                     'default_acceptability_threshold': configdb.get_default_acceptability_threshold(instrument_type),
                     'configuration_types': configdb.get_configuration_types(instrument_type),
                     'camera_type': {
-                        'science_field_of_view': configdb.get_diagnol_ccd_fov(instrument_type, autoguider=False),
-                        'autoguider_field_of_view': configdb.get_diagnol_ccd_fov(instrument_type, autoguider=True),
+                        'science_field_of_view': configdb.get_diagonal_ccd_fov(instrument_type, autoguider=False),
+                        'autoguider_field_of_view': configdb.get_diagonal_ccd_fov(instrument_type, autoguider=True),
                         'pixel_scale': configdb.get_pixel_scale(instrument_type)
                     }
                 }

observation_portal/requestgroups/viewsets.py

@@ -277,10 +277,8 @@ class ConfigurationViewSet(viewsets.GenericViewSet):
 
     @action(detail=False, methods=['post'])
     def dither(self, request):
-        dither_details = request.data
-
         # Check that the dither parameters specified are valid
-        dither_serializer = import_string(settings.SERIALIZERS['requestgroups']['Dither'])(data=dither_details)
+        dither_serializer = self.get_serializer(data=request.data)
         if not dither_serializer.is_valid():
             return Response(dither_serializer.errors, status=400)