Bug in create_genome functionality in process_genome.py does not consider HT (type 5) magnets

[JossWhittle]

In process_genome.py the create_genome flag has the functionality of taking in a human readable genome file and creating a machine readable version of the same data. However, it does not consider HT (type 5) magnets.

https://github.com/DiamondLightSource/Opt-ID/blob/5eae8f15642c98cf762d889715f960801f36258e/IDSort/src/process_genome.py#L194-L233

The human readable input file provided for testing in process_genome_test::test_process_create_genome() does contain HT magnets, but the expected output file was originally generated with code that also had this bug and so does not contain HT magnets for comparison.

Fixing the bug paradoxically causes the test to fail because it now correctly includes HT magnets.

---

In practice during real world usage, the create_genome flag is never used as we never end up with human readable .genome.inp files that we need to convert back into machine readable .genomes during normal operation.

During testing, dedicated files of .genome.inp type are converted to .genome.inp.genome files which erroneously do not contain the HT data. But these files are fundamentally separate from legitimate .genome and .genome.inp files used elsewhere. The bug and erroneous data are therefore isolated to the create_genome flag usage and test case and specifically to process_genome_test::test_process_create_genome().

[JossWhittle]

Commit https://github.com/JossWhittle/Opt-ID/commit/3b588371f3f766bcc087971d5458064a872e07ed on fork fixes the bug and corrects the expected output file used for the test.

https://github.com/JossWhittle/Opt-ID/blob/3b588371f3f766bcc087971d5458064a872e07ed/IDSort/src/process_genome.py#L154-L173

To ensure this new expected output file is correct the following procedure was followed:

• The original code was executed with the original files.
  • The test passed as expected despite containing the error.
• The new code was executed with the original files.
  • The test failed paradoxically as it now considered HT magnets.
• The new code was executed with the original files again, with a condition on Line 163 to skip HT type magnets.
  • The test passed, proving that in the absence of HT magnets the output is correct.
  • Because the new code is generic and does not have a separate code path for each magnet type, we can have confidence it is correct for all magnet types.
• The new code was executed with the original files again without the condition on Line 163, allowing for HT type magnets to be included.
  • The test failed paradoxically as expected.
  • The observed output .genome.inp.genome file which we now trust to actually be correct and includes HT magnet data, is used to replace the original expected output file for the process_genome_test::test_process_create_genome() test case.
• The new code was executed with the new files.
  • The test passes correctly as expected.

The PR that merges this fix from the fork will close the issue.

[markbasham]

@eddrial what are HT magnets again, and are they important? Cheers, Mark

[JossWhittle]

Closed by #57