From 3f235c1b7b2a0cbd29b31e12c6efb0958ecdfb6c Mon Sep 17 00:00:00 2001 From: Michael Walsh Date: Mon, 9 Dec 2024 14:26:10 -0500 Subject: [PATCH] address the last comments I missed --- .../backend/data/DataFactoryService.py | 4 --- src/snapred/backend/data/GroceryService.py | 2 +- src/snapred/backend/data/Indexer.py | 9 ++++-- src/snapred/backend/data/LocalDataService.py | 6 ++-- .../backend/service/NormalizationService.py | 4 ++- .../backend/service/ReductionService.py | 4 ++- .../backend/data/test_DataFactoryService.py | 2 +- tests/unit/backend/data/test_Indexer.py | 32 +++++++++---------- .../service/test_NormalizationService.py | 2 +- .../backend/service/test_ReductionService.py | 8 ++--- 10 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/snapred/backend/data/DataFactoryService.py b/src/snapred/backend/data/DataFactoryService.py index d20558347..776c046d0 100644 --- a/src/snapred/backend/data/DataFactoryService.py +++ b/src/snapred/backend/data/DataFactoryService.py @@ -161,10 +161,6 @@ def getNormalizationDataWorkspace(self, runId: str, useLiteMode: bool, version: def getLatestApplicableNormalizationVersion(self, runId: str, useLiteMode: bool): return self.lookupService.normalizationIndexer(runId, useLiteMode).latestApplicableVersion(runId) - @validate_call - def getLatestNormalizationVersion(self, runId: str, useLiteMode: bool): - return self.lookupService.normalizationIndexer(runId, useLiteMode).latestApplicableVersion(runId) - ##### REDUCTION METHODS ##### @validate_call diff --git a/src/snapred/backend/data/GroceryService.py b/src/snapred/backend/data/GroceryService.py index 4bc472fe9..258f83f58 100644 --- a/src/snapred/backend/data/GroceryService.py +++ b/src/snapred/backend/data/GroceryService.py @@ -341,7 +341,7 @@ def createDiffcalTableWorkspaceName( """ wsName = wng.diffCalTable().runNumber(runNumber).version(version).build() if version in [VersionState.DEFAULT, VERSION_START]: - wsName = wsName = wng.diffCalTable().runNumber("default").version(VersionState.DEFAULT).build() + wsName = wng.diffCalTable().runNumber("default").version(VersionState.DEFAULT).build() return wsName @validate_call diff --git a/src/snapred/backend/data/Indexer.py b/src/snapred/backend/data/Indexer.py index d359aa75c..aa908c235 100644 --- a/src/snapred/backend/data/Indexer.py +++ b/src/snapred/backend/data/Indexer.py @@ -126,7 +126,12 @@ def reconcileIndexToFiles(self): f"The following records were expected, but not available on disk: {missingRecords}" ) else: - logger.warning(f"The following records were expected, but not available on disk: {missingRecords}") + logger.warning( + ( + f"The following records were expected, but not available on disk: {missingRecords}", + "\n Please contact your IS or CIS about these missing records.", + ) + ) # take the set of versions common to both commonVersions = self.dirVersions & indexVersions @@ -363,7 +368,7 @@ def _flattenVersion(self, version: Version): def versionExists(self, version: Version): return self._flattenVersion(version) in self.index - def writeNewRecord(self, record: Record, entry: IndexEntry): + def writeNewVersion(self, record: Record, entry: IndexEntry): """ Coupled write of a record and an index entry. As required for new records. diff --git a/src/snapred/backend/data/LocalDataService.py b/src/snapred/backend/data/LocalDataService.py index 2d7b5d00f..4ef85944a 100644 --- a/src/snapred/backend/data/LocalDataService.py +++ b/src/snapred/backend/data/LocalDataService.py @@ -525,7 +525,7 @@ def writeNormalizationRecord(self, record: NormalizationRecord, entry: Optional[ if entry is None: indexer.writeRecord(record) else: - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) # separately write the normalization state indexer.writeParameters(record.calculationParameters) @@ -587,7 +587,7 @@ def writeCalibrationRecord(self, record: CalibrationRecord, entry: Optional[Inde indexer.writeRecord(record) else: # write record to file - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) # separately write the calibration state indexer.writeParameters(record.calculationParameters) @@ -1034,7 +1034,7 @@ def initializeState(self, runId: str, useLiteMode: bool, name: str = None): comments="The default configuration when loading StateConfig if none other is found", ) # write the calibration state - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) indexer.writeParameters(record.calculationParameters) # write the default diffcal table self._writeDefaultDiffCalTable(runId, liteMode) diff --git a/src/snapred/backend/service/NormalizationService.py b/src/snapred/backend/service/NormalizationService.py index 585cb771d..9c20ff0a1 100644 --- a/src/snapred/backend/service/NormalizationService.py +++ b/src/snapred/backend/service/NormalizationService.py @@ -391,7 +391,9 @@ def matchRunsToNormalizationVersions(self, request: MatchRunsRequest) -> Dict[st """ response = {} for runNumber in request.runNumbers: - response[runNumber] = self.dataFactoryService.getLatestNormalizationVersion(runNumber, request.useLiteMode) + response[runNumber] = self.dataFactoryService.getLatestApplicableNormalizationVersion( + runNumber, request.useLiteMode + ) return response @FromString diff --git a/src/snapred/backend/service/ReductionService.py b/src/snapred/backend/service/ReductionService.py index ceaa0a798..29adfe657 100644 --- a/src/snapred/backend/service/ReductionService.py +++ b/src/snapred/backend/service/ReductionService.py @@ -367,7 +367,9 @@ def fetchReductionGroceries(self, request: ReductionRequest) -> Dict[str, Any]: request.runNumber, request.useLiteMode ) if ContinueWarning.Type.MISSING_NORMALIZATION not in request.continueFlags: - normVersion = self.dataFactoryService.getLatestNormalizationVersion(request.runNumber, request.useLiteMode) + normVersion = self.dataFactoryService.getLatestApplicableNormalizationVersion( + request.runNumber, request.useLiteMode + ) # Fetch pixel masks residentMasks = {} diff --git a/tests/unit/backend/data/test_DataFactoryService.py b/tests/unit/backend/data/test_DataFactoryService.py index ab9000e2b..8d4423ce6 100644 --- a/tests/unit/backend/data/test_DataFactoryService.py +++ b/tests/unit/backend/data/test_DataFactoryService.py @@ -228,7 +228,7 @@ def test_getNormalizationDataWorkspace(self): def test_getLatestNormalizationVersion(self): for useLiteMode in [True, False]: - actual = self.instance.getLatestNormalizationVersion("123", useLiteMode) + actual = self.instance.getLatestApplicableNormalizationVersion("123", useLiteMode) assert actual == self.expected("Normalization", "123") ## TEST REDUCTION METHODS diff --git a/tests/unit/backend/data/test_Indexer.py b/tests/unit/backend/data/test_Indexer.py index 734b02aba..04eacf00d 100644 --- a/tests/unit/backend/data/test_Indexer.py +++ b/tests/unit/backend/data/test_Indexer.py @@ -282,7 +282,7 @@ def test_flattenVersion(self): with pytest.raises(ValueError, match=r".*Version must be an int or*"): indexer._flattenVersion(None) - def test_writeNewRecord_noAppliesTo(self): + def test_writeNewVersion_noAppliesTo(self): # ensure that a new record is written to disk # and the index is updated to reflect the new record indexer = self.initIndexer() @@ -290,18 +290,18 @@ def test_writeNewRecord_noAppliesTo(self): record = self.record(version) entry = self.indexEntryFromRecord(record) entry.appliesTo = None - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert self.recordPath(version).exists() assert indexer.index[version] == entry - def test_writeNewRecord_recordAlreadyExists(self): + def test_writeNewVersion_recordAlreadyExists(self): # ensure that a new record is written to disk # and the index is updated to reflect the new record indexer = self.initIndexer() version = randint(2, 120) record = self.record(version) entry = self.indexEntryFromRecord(record) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert self.recordPath(version).exists() assert indexer.index[version] == entry @@ -311,7 +311,7 @@ def test_writeNewRecord_recordAlreadyExists(self): entry = self.indexEntryFromRecord(record) with pytest.raises(ValueError, match=".*already exists.*"): - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) def test_currentVersion_add(self): # ensure current version advances when index entries are written @@ -509,7 +509,7 @@ def test_nextVersion(self): # now write the record # >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> WRITE 1 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< record = self.recordFromIndexEntry(entry) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) expectedIndex[here] = entry # the current version hasn't moved @@ -550,7 +550,7 @@ def test_nextVersion(self): record = self.record(here) entry = self.indexEntryFromRecord(record) expectedIndex[entry.version] = entry - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert indexer.nextVersion() == here + 1 assert indexer.nextVersion() not in indexer.index @@ -579,7 +579,7 @@ def test_nextVersion_with_default_index_first(self): entry = self.indexEntry(VersionState.DEFAULT) record = self.recordFromIndexEntry(entry) expectedIndex[VERSION_START] = entry - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert self.recordPath(indexer.defaultVersion()).exists() # the current version is still the default version assert indexer.currentVersion() == indexer.defaultVersion() @@ -601,7 +601,7 @@ def test_nextVersion_with_default_index_first(self): # now write the record -- ensure it is written at the record = self.recordFromIndexEntry(entry) entry = self.indexEntryFromRecord(record) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert self.recordPath(VERSION_START).exists() # ensure current still here assert indexer.currentVersion() == VERSION_START + 1 @@ -624,7 +624,7 @@ def test_nextVersion_with_default_record_first(self): record = self.record(VersionState.DEFAULT) entry = self.indexEntryFromRecord(record) assert indexer._flattenVersion(record.version) == VERSION_START - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) # the current version is still the default version assert indexer.currentVersion() == VERSION_START @@ -818,7 +818,7 @@ def test_readWriteRecord_next_version(self): # and the read / written records match record = self.record(nextVersion) entry = self.indexEntryFromRecord(record) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) res = indexer.readRecord(nextVersion) assert record.version == nextVersion assert res == record @@ -832,7 +832,7 @@ def test_readWriteRecord_any_version(self): # write then read the record # make sure the record version was updated # and the read / written records match - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) res = indexer.readRecord(version) assert record.version == version assert res == record @@ -870,7 +870,7 @@ def test_writeRecord_with_version(self): record = self.record(version) entry = self.indexEntryFromRecord(record) indexer = self.initIndexer() - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert record.version == version assert self.recordPath(version).exists() # read it back in and ensure it is the same @@ -892,7 +892,7 @@ def test_writeRecord_next_version(self): # now write the record record = self.record(nextVersion) entry = self.indexEntryFromRecord(record) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) assert record.version == nextVersion assert self.recordPath(nextVersion).exists() res = parse_file_as(Record, self.recordPath(nextVersion)) @@ -910,7 +910,7 @@ def test_readWriteRecord_calibration(self): entry = self.indexEntryFromRecord(record) # write then read in the record indexer = self.initIndexer(IndexerType.CALIBRATION) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) res = indexer.readRecord(record.version) assert type(res) is CalibrationRecord assert res == record @@ -922,7 +922,7 @@ def test_readWriteRecord_normalization(self): entry = self.indexEntryFromRecord(record) # write then read in the record indexer = self.initIndexer(IndexerType.NORMALIZATION) - indexer.writeNewRecord(record, entry) + indexer.writeNewVersion(record, entry) res = indexer.readRecord(record.version) assert type(res) is NormalizationRecord assert res == record diff --git a/tests/unit/backend/service/test_NormalizationService.py b/tests/unit/backend/service/test_NormalizationService.py index 012ad822d..dbc1076ec 100644 --- a/tests/unit/backend/service/test_NormalizationService.py +++ b/tests/unit/backend/service/test_NormalizationService.py @@ -211,7 +211,7 @@ def test_smoothDataExcludingPeaks( ) def test_matchRuns(self): - self.instance.dataFactoryService.getLatestNormalizationVersion = mock.Mock( + self.instance.dataFactoryService.getLatestApplicableNormalizationVersion = mock.Mock( side_effect=[mock.sentinel.version1, mock.sentinel.version2], ) request = mock.Mock(runNumbers=[mock.sentinel.run1, mock.sentinel.run2], useLiteMode=True) diff --git a/tests/unit/backend/service/test_ReductionService.py b/tests/unit/backend/service/test_ReductionService.py index 55a769e7b..c92f95856 100644 --- a/tests/unit/backend/service/test_ReductionService.py +++ b/tests/unit/backend/service/test_ReductionService.py @@ -126,7 +126,7 @@ def test_prepReductionIngredients(self): def test_fetchReductionGroceries(self): self.instance.dataFactoryService.getLatestApplicableCalibrationVersion = mock.Mock(return_value=1) - self.instance.dataFactoryService.getLatestNormalizationVersion = mock.Mock(return_value=1) + self.instance.dataFactoryService.getLatestApplicableNormalizationVersion = mock.Mock(return_value=1) self.instance._markWorkspaceMetadata = mock.Mock() self.request.continueFlags = ContinueWarning.Type.UNSET res = self.instance.fetchReductionGroceries(self.request) @@ -145,7 +145,7 @@ def test_reduction(self, mockReductionRecipe): self.instance.dataFactoryService.getLatestApplicableCalibrationVersion = mock.Mock(return_value=1) self.instance.dataFactoryService.stateExists = mock.Mock(return_value=True) self.instance.dataFactoryService.calibrationExists = mock.Mock(return_value=True) - self.instance.dataFactoryService.getLatestNormalizationVersion = mock.Mock(return_value=1) + self.instance.dataFactoryService.getLatestApplicableNormalizationVersion = mock.Mock(return_value=1) self.instance.dataFactoryService.normalizationExists = mock.Mock(return_value=True) self.instance._markWorkspaceMetadata = mock.Mock() @@ -681,7 +681,7 @@ def trackFetchGroceryDict(*args, **kwargs): focusGroups=[FocusGroup(name="apple", definition="path/to/grouping")], ) self.service.dataFactoryService.getLatestApplicableCalibrationVersion = mock.Mock(return_value=1) - self.service.dataFactoryService.getLatestNormalizationVersion = mock.Mock(return_value=2) + self.service.dataFactoryService.getLatestApplicableNormalizationVersion = mock.Mock(return_value=2) self.service._markWorkspaceMetadata = mock.Mock() groceryClerk = self.service.groceryClerk @@ -742,7 +742,7 @@ def test_fetchReductionGroceries_pixelMasks_not_a_mask(self): focusGroups=[FocusGroup(name="apple", definition="path/to/grouping")], ) self.service.dataFactoryService.getLatestApplicableCalibrationVersion = mock.Mock(return_value=1) - self.service.dataFactoryService.getLatestNormalizationVersion = mock.Mock(return_value=2) + self.service.dataFactoryService.getLatestApplicableNormalizationVersion = mock.Mock(return_value=2) combinedMaskName = wng.reductionPixelMask().runNumber(request.runNumber).build() mockPrepCombinedMask.return_value = combinedMaskName