Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor indexer to rounte saves and loads through a generic Versione… #540

Draft
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

walshmm
Copy link
Collaborator

@walshmm walshmm commented Jan 30, 2025

…dObject form of each method

Description of work

Explanation of work

To test

Dev testing

CIS testing

# replace with your test script below

Link to EWM item

EWM#<ticket_number>

Verification

  • the author has read the EWM story and acceptance critera
  • the reviewer has read the EWM story and acceptance criteria
  • the reviewer certifies the acceptance criteria below reflect the criteria in EWM

Acceptance Criteria

This list is for ease of reference, and does not replace reading the EWM story as part of the review. Verify this list matches the EWM story before reviewing.

  • acceptance criterion 1
  • acceptance criterion 2

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 79.48718% with 8 lines in your changes missing coverage. Please review.

Project coverage is 95.75%. Comparing base (9eb99a8) to head (e186da8).

Files with missing lines Patch % Lines
src/snapred/backend/data/Indexer.py 85.71% 5 Missing ⚠️
src/snapred/backend/data/LocalDataService.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next     #540      +/-   ##
==========================================
- Coverage   95.98%   95.75%   -0.23%     
==========================================
  Files          68       68              
  Lines        5152     5164      +12     
==========================================
  Hits         4945     4945              
- Misses        207      219      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +930 to +987
# def test_readParameters_nope(self):
# indexer = self.initIndexer()
# assert not indexer.parametersPath(1).exists()
# with pytest.raises(FileNotFoundError):
# indexer.readParameters(1)

# def test_readWriteParameters(self):
# version = randint(1, 10)
# params = self.calculationParameters(version)

# indexer = self.initIndexer()
# indexer.writeParameters(params)
# res = indexer.readParameters(version)
# assert res.version == version
# assert res == params

# def test_readWriteParameters_warn_overwrite(self):
# version = randint(1, 100)

# indexer = self.initIndexer()

# # write some parameters at a version
# params1 = self.calculationParameters(version)
# indexer.writeParameters(params1)
# assert indexer.parametersPath(version).exists()

# # now try to overwrite parameters at same version
# # make sure a warning is logged
# with self.assertLogs(logger=IndexerModule.logger, level=logging.WARNING) as cm:
# params2 = self.calculationParameters(version)
# indexer.writeParameters(params2)
# assert f"Overwriting parameters at {indexer.parametersPath(version)}" in cm.output[0]

# # make sure the indexer can read/write specific state parameter types #

# def test_readWriteParameters_calibration(self):
# params = DAOFactory.calibrationParameters()
# indexer = self.initIndexer(IndexerType.CALIBRATION)
# indexer.writeParameters(params)
# res = indexer.readParameters(params.version)
# assert type(res) is Calibration
# assert res == params

# def test_readWriteParameters_normalization(self):
# params = DAOFactory.normalizationParameters()
# indexer = self.initIndexer(IndexerType.NORMALIZATION)
# indexer.writeParameters(params)
# res = indexer.readParameters(params.version)
# assert type(res) is Normalization
# assert res == params

# def test_readWriteParameters_reduction(self):
# params = CalculationParameters(**DAOFactory.calibrationParameters().model_dump())
# indexer = self.initIndexer(IndexerType.REDUCTION)
# indexer.writeParameters(params)
# res = indexer.readParameters(params.version)
# assert type(res) is CalculationParameters
# assert res == params
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an oversight that would let parameters write w/o there being a valid index entry, it would just create its desired folders. These tests are commented out because read/writeParameters just calls the same function as everything else now, so these test are redundant/will be rewritten in the context of Save/LoadVersionedObject if novel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant