Increased robustness of opconvertor.py when running on some experimental data - #456
Merged
Merged
Conversation
- OP data for some experiments is lacking standard deviation values leading to an IndexError, added an additional length check - added a check wether the atom is actually contained in the mapping dict to avoid a KeyError, missing atoms are skipped - 'nice_OPdict' contain a lot of NaN, added a function to replace those because it almost certainly will be dumped to JSON
- Until we decide on test data, it cannot be run
comcon1
requested changes
Feb 10, 2026
Comment on lines
+96
to
+97
| fairmd.lipids.FMDL_DATA_PATH = "../../BilayerData" | ||
| experiments = ExperimentCollection.load_from_data("OPExperiment") |
Comment on lines
+80
to
+81
| @pytest.mark.skipif(IS_CI, reason="This test is skipped on GitHub Actions") | ||
| def test_build_nice_OPdict_with_all_experiments(self, monkeypatch, tmpdir): |
Member
There was a problem hiding this comment.
Add fixture loading experiments. Add 2 experiments to ToyData.
| - name: run Python tests | ||
| run: | | ||
| tox -e tests-min | ||
| tox -e tests-min -r |
| - name: run Python tests | ||
| run: | | ||
| tox -e tests-all | ||
| tox -e tests-all -r |
Comment on lines
+66
to
+68
| idx = int(match[1]) | ||
| row["H"] = str(idx) | ||
| if match and len(match.groups()) == 1: | ||
| idx = int(match[1]) | ||
| row["H"] = str(idx) |
Collaborator
Author
There was a problem hiding this comment.
Hi @comcon1 I reverted this, but it is no good. Without the safety check, 209 of the Simulation fail loading op data. I think the way it is now is the most unsafe way. We need to handle this case that there is no match more defensively. Or we need to correct all the OP data.
Comment on lines
+92
to
+93
| if atom_c not in mdict: | ||
| continue |
| check.is_true(has_c("g3", rdict["glycerol backbone"])) | ||
|
|
||
| # Check both common CI indicators | ||
| IS_CI = os.getenv("GITHUB_ACTIONS") == "true" or os.getenv("CI") == "true" |
| # Check both common CI indicators | ||
| IS_CI = os.getenv("GITHUB_ACTIONS") == "true" or os.getenv("CI") == "true" | ||
|
|
||
| @pytest.mark.skipif(IS_CI, reason="This test is skipped on GitHub Actions") |
| # Validate data types | ||
| assert isinstance(entry["C"], str), "C atom name is not string" | ||
| assert isinstance(entry["H"], str), "H atom name is not string" | ||
| assert isinstance(entry["OP"], (int, float)) or entry["OP"] is None, "OP is not numeric" |
matched by the renamer. We are now throwing a ValueException with speaking error message. This is a more defensive approach than simply risking NoneType is not subsettable errors.
comcon1
approved these changes
Mar 1, 2026
comcon1
left a comment
comcon1
left a comment
Member
There was a problem hiding this comment.
Will approve and merge this. Then I will work to add functionality in a separate PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Made build_nice_OPdict more robust in corner cases.
📚 Documentation preview 📚: https://databank--456.org.readthedocs.build/