From 46efe9007d2112da1cb796c8248290db16409189 Mon Sep 17 00:00:00 2001 From: Jon Date: Fri, 20 Dec 2019 07:11:55 +0000 Subject: [PATCH 1/2] switch the fill_window attribute to act on the configuration instead of instrument_config level --- .../migrations/0013_auto_20191220_0711.py | 24 ++++ observation_portal/requestgroups/models.py | 9 +- .../requestgroups/serializers.py | 34 +++-- .../requestgroups/test/test_api.py | 116 +++++++----------- 4 files changed, 90 insertions(+), 93 deletions(-) create mode 100644 observation_portal/requestgroups/migrations/0013_auto_20191220_0711.py diff --git a/observation_portal/requestgroups/migrations/0013_auto_20191220_0711.py b/observation_portal/requestgroups/migrations/0013_auto_20191220_0711.py new file mode 100644 index 00000000..da2cfdfd --- /dev/null +++ b/observation_portal/requestgroups/migrations/0013_auto_20191220_0711.py @@ -0,0 +1,24 @@ +# Generated by Django 2.2.4 on 2019-12-20 07:11 + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('requestgroups', '0012_auto_20191123_0233'), + ] + + operations = [ + migrations.AlterField( + model_name='configuration', + name='repeat_duration', + field=models.FloatField(blank=True, help_text='The requested duration for this configuration to be repeated within. Only applicable to REPEAT_* type configurations. Setting parameter fill_window to True will cause this value to automatically be filled in to the max possible given its visibility within the observing window.', null=True, validators=[django.core.validators.MinValueValidator(0.0)], verbose_name='configuration duration'), + ), + migrations.AlterField( + model_name='instrumentconfig', + name='exposure_count', + field=models.PositiveIntegerField(help_text='The number of exposures to take. This field must be set to a value greater than 0.', validators=[django.core.validators.MinValueValidator(1)]), + ), + ] diff --git a/observation_portal/requestgroups/models.py b/observation_portal/requestgroups/models.py index 4c08e0a0..fab384c7 100644 --- a/observation_portal/requestgroups/models.py +++ b/observation_portal/requestgroups/models.py @@ -370,7 +370,9 @@ class Configuration(models.Model): null=True, validators=[MinValueValidator(0.0)], help_text='The requested duration for this configuration to be repeated within. ' - 'Only applicable to REPEAT_* type configurations.' + 'Only applicable to REPEAT_* type configurations. Setting parameter fill_window ' + 'to True will cause this value to automatically be filled in to the max ' + 'possible given its visibility within the observing window.' ) extra_params = JSONField( @@ -619,10 +621,7 @@ class InstrumentConfig(models.Model): exposure_count = models.PositiveIntegerField( validators=[MinValueValidator(1)], # TODO: Update help text. Maybe move fill window instructions to a better place and improve them. - help_text='The number of exposures to take. This field must be set to a value greater than 0, but optionally ' - 'you can add a boolean fill_window field to the Configuration upon submission with a value ' - 'of true. If this is set, the exposure_count will be set to the number of exposures (including ' - 'overheads) that will fit in the observing window.' + help_text='The number of exposures to take. This field must be set to a value greater than 0.' ) bin_x = models.PositiveSmallIntegerField( verbose_name='y binning', default=1, blank=True, diff --git a/observation_portal/requestgroups/serializers.py b/observation_portal/requestgroups/serializers.py index 0a6877c8..59016df0 100644 --- a/observation_portal/requestgroups/serializers.py +++ b/observation_portal/requestgroups/serializers.py @@ -144,7 +144,6 @@ def validate(self, data): class InstrumentConfigSerializer(serializers.ModelSerializer): - fill_window = serializers.BooleanField(required=False, write_only=True) rois = RegionOfInterestSerializer(many=True, required=False) class Meta: @@ -214,6 +213,7 @@ def validate(self, data): class ConfigurationSerializer(serializers.ModelSerializer): + fill_window = serializers.BooleanField(required=False, write_only=True) constraints = ConstraintsSerializer() instrument_configs = InstrumentConfigSerializer(many=True) acquisition_config = AcquisitionConfigSerializer() @@ -234,8 +234,6 @@ def to_representation(self, instance): return data def validate_instrument_configs(self, value): - if [instrument_config.get('fill_window', False) for instrument_config in value].count(True) > 1: - raise serializers.ValidationError(_('Only one instrument_config can have `fill_window` set')) if len(set([instrument_config.get('rotator_mode', '') for instrument_config in value])) > 1: raise serializers.ValidationError(_('Rotator modes within the same configuration must be the same')) return value @@ -549,6 +547,10 @@ def validate_configurations(self, value): if not value: raise serializers.ValidationError(_('You must specify at least 1 configuration')) + # Only one configuration can have the fill_window attribute set + if [config.get('fill_window', False) for config in value].count(True) > 1: + raise serializers.ValidationError(_('Only one configuration can have `fill_window` set')) + constraints = value[0]['constraints'] # Set the relative priority of molecules in order for i, configuration in enumerate(value): @@ -615,22 +617,16 @@ def validate(self, data): rise_set_intervals_by_site = get_filtered_rise_set_intervals_by_site(data, is_staff=is_staff) largest_interval = get_largest_interval(rise_set_intervals_by_site) for configuration in data['configurations']: - for instrument_config in configuration['instrument_configs']: - if instrument_config.get('fill_window'): - instrument_config_duration = get_instrument_configuration_duration( - instrument_config, configuration['instrument_type'] - ) - num_exposures = get_num_exposures( - instrument_config, configuration['instrument_type'], - largest_interval - timedelta(seconds=duration - instrument_config_duration) - ) - instrument_config['exposure_count'] = num_exposures - duration = get_request_duration(data) - # delete the fill window attribute, it is only used for this validation - try: - del instrument_config['fill_window'] - except KeyError: - pass + if 'REPEAT' in configuration['type'].upper() and configuration.get('fill_window'): + max_configuration_duration = largest_interval.total_seconds() - duration + configuration.get('repeat_duration', 0) - 1 + configuration['repeat_duration'] = max_configuration_duration + duration = get_request_duration(data) + + # delete the fill window attribute, it is only used for this validation + try: + del configuration['fill_window'] + except KeyError: + pass if largest_interval.total_seconds() <= 0: raise serializers.ValidationError( _( diff --git a/observation_portal/requestgroups/test/test_api.py b/observation_portal/requestgroups/test/test_api.py index 5834cd32..351148b5 100644 --- a/observation_portal/requestgroups/test/test_api.py +++ b/observation_portal/requestgroups/test/test_api.py @@ -1605,63 +1605,64 @@ def test_configurations_automatically_have_priority_set(self): for i, configuration in enumerate(rg['requests'][0]['configurations']): self.assertEqual(configuration['priority'], i + 1) - def test_fill_window_on_more_than_one_instrument_config_fails(self): + def test_fill_window_on_more_than_one_configuration_fails(self): bad_data = self.generic_payload.copy() - bad_data['requests'][0]['configurations'][0]['instrument_configs'].append( - self.extra_configuration['instrument_configs'][0].copy() + bad_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + bad_data['requests'][0]['configurations'][0]['repeat_duration'] = 250 + bad_data['requests'][0]['configurations'].append( + bad_data['requests'][0]['configurations'][0].copy() ) - bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - bad_data['requests'][0]['configurations'][0]['instrument_configs'][1]['fill_window'] = True + bad_data['requests'][0]['configurations'][0]['fill_window'] = True + bad_data['requests'][0]['configurations'][1]['fill_window'] = True response = self.client.post(reverse('api:request_groups-list'), data=bad_data) - self.assertIn('Only one instrument_config can have `fill_window` set', str(response.content)) + self.assertIn('Only one configuration can have `fill_window` set', str(response.content)) self.assertEqual(response.status_code, 400) def test_fill_window_one_configuration_fills_the_window(self): good_data = self.generic_payload.copy() - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - initial_exposure_count = good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'] - response = self.client.post(reverse('api:request_groups-list'), data=good_data) - rg = response.json() - self.assertGreater(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], - initial_exposure_count) - self.assertEqual(response.status_code, 201) - - def test_fill_window_two_instrument_configs_one_false_fills_the_window(self): - good_data = self.generic_payload.copy() - good_data['requests'][0]['configurations'][0]['instrument_configs'].append( - self.extra_configuration['instrument_configs'][0].copy() - ) - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - good_data['requests'][0]['configurations'][0]['instrument_configs'][1]['fill_window'] = False - initial_exposure_count = good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'] + initial_repeat_duration = 250 + good_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + good_data['requests'][0]['configurations'][0]['repeat_duration'] = initial_repeat_duration + good_data['requests'][0]['configurations'][0]['fill_window'] = True response = self.client.post(reverse('api:request_groups-list'), data=good_data) rg = response.json() - self.assertGreater(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], - initial_exposure_count) + self.assertGreater(rg['requests'][0]['configurations'][0]['repeat_duration'], + initial_repeat_duration) self.assertEqual(response.status_code, 201) - def test_fill_window_two_instrument_configs_one_blank_fills_the_window(self): + def test_fill_window_two_configs_one_false_fills_the_window(self): good_data = self.generic_payload.copy() - good_data['requests'][0]['configurations'][0]['instrument_configs'].append( - self.extra_configuration['instrument_configs'][0].copy() + initial_repeat_duration = 250 + good_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + good_data['requests'][0]['configurations'][0]['repeat_duration'] = initial_repeat_duration + good_data['requests'][0]['configurations'].append( + good_data['requests'][0]['configurations'][0].copy() ) - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - initial_exposure_count = good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'] + good_data['requests'][0]['configurations'][0]['fill_window'] = True + good_data['requests'][0]['configurations'][1]['fill_window'] = False response = self.client.post(reverse('api:request_groups-list'), data=good_data) rg = response.json() - self.assertGreater(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], initial_exposure_count) + self.assertGreater(rg['requests'][0]['configurations'][0]['repeat_duration'], + initial_repeat_duration) + self.assertEqual(rg['requests'][0]['configurations'][1]['repeat_duration'], + initial_repeat_duration) self.assertEqual(response.status_code, 201) - def test_fill_window_two_instrument_configs_first_fills_the_window(self): + def test_fill_window_two_configs_one_blank_fills_the_window(self): good_data = self.generic_payload.copy() - good_data['requests'][0]['configurations'][0]['instrument_configs'].append( - self.extra_configuration['instrument_configs'][0].copy() + initial_repeat_duration = 250 + good_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + good_data['requests'][0]['configurations'][0]['repeat_duration'] = initial_repeat_duration + good_data['requests'][0]['configurations'].append( + good_data['requests'][0]['configurations'][0].copy() ) - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - initial_exposure_count = good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'] + good_data['requests'][0]['configurations'][0]['fill_window'] = True response = self.client.post(reverse('api:request_groups-list'), data=good_data) rg = response.json() - self.assertGreater(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], initial_exposure_count) + self.assertGreater(rg['requests'][0]['configurations'][0]['repeat_duration'], + initial_repeat_duration) + self.assertEqual(rg['requests'][0]['configurations'][1]['repeat_duration'], + initial_repeat_duration) self.assertEqual(response.status_code, 201) def test_fill_window_not_enough_time_fails(self): @@ -1670,52 +1671,29 @@ def test_fill_window_not_enough_time_fails(self): 'start': '2016-09-29T21:12:18Z', 'end': '2016-09-29T21:21:19Z' } - bad_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True + bad_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + bad_data['requests'][0]['configurations'][0]['repeat_duration'] = 250 + bad_data['requests'][0]['configurations'][0]['fill_window'] = True response = self.client.post(reverse('api:request_groups-list'), data=bad_data) self.assertIn('the target is never visible within the time window', str(response.content)) self.assertEqual(response.status_code, 400) def test_fill_window_confined_window_fills_the_window(self): good_data = self.generic_payload.copy() + initial_repeat_duration = 250 good_data['requests'][0]['windows'][0] = { 'start': '2016-09-29T23:12:18Z', 'end': '2016-09-29T23:21:19Z' } - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - response = self.client.post(reverse('api:request_groups-list'), data=good_data) - rg = response.json() - self.assertEqual(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], 3) - self.assertEqual(response.status_code, 201) - - def test_fill_window_confined_window_2_fills_the_window(self): - good_data = self.generic_payload.copy() - good_data['requests'][0]['windows'][0] = { - 'start': '2016-09-29T23:12:18Z', - 'end': '2016-09-29T23:21:19Z' - } - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_time'] = 50 - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True + good_data['requests'][0]['configurations'][0]['type'] = 'REPEAT_EXPOSE' + good_data['requests'][0]['configurations'][0]['repeat_duration'] = initial_repeat_duration + good_data['requests'][0]['configurations'][0]['fill_window'] = True response = self.client.post(reverse('api:request_groups-list'), data=good_data) rg = response.json() - self.assertEqual(rg['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], 5) - self.assertEqual(response.status_code, 201) - - def test_fill_window_when_exposures_fit_exactly(self): - good_data = self.generic_payload.copy() - good_data['requests'][0]['windows'][0] = { - 'start': '2016-09-29T23:12:00Z', - 'end': '2016-09-29T23:21:30Z' - } - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_time'] = 10 - good_data['requests'][0]['configurations'][0]['instrument_configs'][0]['fill_window'] = True - # The largest interval is 570s, available time is 460s, and instrument config duration is 46s - n_exposures_fit_exactly = 10 - response = self.client.post(reverse('api:request_groups-list'), data=good_data) + self.assertGreater(rg['requests'][0]['configurations'][0]['repeat_duration'], + initial_repeat_duration) + self.assertEqual(rg['requests'][0]['configurations'][0]['repeat_duration'], 430.0) self.assertEqual(response.status_code, 201) - self.assertEqual( - response.json()['requests'][0]['configurations'][0]['instrument_configs'][0]['exposure_count'], - n_exposures_fit_exactly - 1 - ) def test_multiple_instrument_configs_with_different_rotator_modes_fails(self): bad_data = self.generic_payload.copy() From 1e24078a3ef2fb4641000914e3401d1ecdf31668 Mon Sep 17 00:00:00 2001 From: Jon Date: Sat, 21 Dec 2019 01:02:41 +0000 Subject: [PATCH 2/2] remove comment --- observation_portal/requestgroups/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/observation_portal/requestgroups/models.py b/observation_portal/requestgroups/models.py index fab384c7..8297adfe 100644 --- a/observation_portal/requestgroups/models.py +++ b/observation_portal/requestgroups/models.py @@ -620,7 +620,6 @@ class InstrumentConfig(models.Model): ) exposure_count = models.PositiveIntegerField( validators=[MinValueValidator(1)], - # TODO: Update help text. Maybe move fill window instructions to a better place and improve them. help_text='The number of exposures to take. This field must be set to a value greater than 0.' ) bin_x = models.PositiveSmallIntegerField(