Generate AfidSet for testing #24
Conversation
- Add a strategy that updates afids_coords to generating a near-complete AfidSet. `desc` was left out as it would not be read upon saving and this field may be dropped in future updates. - Additionally adds a round-trip AfidSet.load, asserting that the file loaded is of the correct class in `save_fcsv`
kaitj
added
the
maintenance
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 8 8
Lines 348 353 +5
=========================================
+ Hits 348 353 +5
|
| @@ -19,7 +19,7 @@ class AfidPosition: | |||
| x: float = attrs.field() | |||
| y: float = attrs.field() | |||
| z: float = attrs.field() | |||
| desc: str = attrs.field() | |||
| desc: str = attrs.field(default="") | |||
There was a problem hiding this comment.
I'd rather not have a default value that's invalid here (even if you want to leave the invalid descs in the test for now)
There was a problem hiding this comment.
Fair enough, I think alternatively I can load in the valid descs and any invalid ones set to "Unknown".
There was a problem hiding this comment.
Sounds reasonable to me, or you can just leave it without a default until we implement the validation
| def test_invalid_num_afids(self, afids_coords: list[AfidPosition]) -> None: | ||
| # Check to see if file can be loaded with afids-utils | ||
| test_load = AfidSet.load(out_fcsv_file.name) | ||
| assert isinstance(test_load, AfidSet) |
There was a problem hiding this comment.
I think we could even assert test_load == afids_set
There was a problem hiding this comment.
Ahh yeah, this test can be simplified quite a bit with the new afid_sets strategy (and slight modification since it also randomly generates the slicer version by default)
|
Oh nvm, I see. it is because #25 got merged in and that was based off of this one. Oops. Will make a new patch PR to address the comments |
Proposed changes
This PR updates and renames the
afid_coordstrategy toafid_set, generating a near completeAfidSetobject (sans desc). This allows for future testing of methods requiring a (in)validAfidSet, while still being able to use a list ofAfidPositionsby invokingAfidSet.afids.Additionally, this adds in an additional
AfidSet.loadto thevalid_save_fcsvtest, to ensure that the generated AfidSet that is saved can also be loaded.Types of changes
What types of changes does your code introduce? Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!poe qualitytaskNotes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.