Skip to content

Commit

Permalink
Check that qc_record.extras exists before accessing it (#370)
Browse files Browse the repository at this point in the history
* factor out _get_qc_record_id

* make sure qc_record.extras exists before accessing it

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* update release notes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
ntBre and pre-commit-ci[bot] authored Oct 31, 2024
1 parent a7cfc91 commit 8c4bad6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 16 deletions.
4 changes: 4 additions & 0 deletions docs/releasehistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Releases follow the ``major.minor.micro`` scheme recommended by
* `micro` increments represent bugfix releases or improvements in documentation

<!-- ## Since last release -->
## Current Development
### Bug fixes
* [#370] - Check that the `extras` field on QCArchive records is not `None` before accessing it to avoid crashes [@ntBre]

## 0.4.0 / 19-09-2024

Expand Down Expand Up @@ -171,6 +174,7 @@ The first major release of bespokefit intended for public use.
[#334]: https://github.com/openforcefield/openff-bespokefit/pull/334
[#338]: https://github.com/openforcefield/openff-bespokefit/pull/338
[#351]: https://github.com/openforcefield/openff-bespokefit/pull/351
[#370]: https://github.com/openforcefield/openff-bespokefit/pull/370


[@Yoshanuikabundi]: https://github.com/Yoshanuikabundi
Expand Down
29 changes: 13 additions & 16 deletions openff/bespokefit/optimizers/forcebalance/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ def _standardize_grid_id_str(grid_id: str) -> tuple[Union[int, float]]:
return tuple(grid_id)


def _get_qc_record_id(qc_record):
return (
qc_record.extras["id"]
if qc_record.extras and "id" in qc_record.extras
else qc_record.id
)


class _TargetFactory(Generic[T], abc.ABC):
@classmethod
@abc.abstractmethod
Expand Down Expand Up @@ -117,11 +125,8 @@ def _batch_qc_records(
target.
"""

def qc_record_id(qc_record):
return qc_record.extras["id"] if "id" in qc_record.extras else qc_record.id

return {
f"{cls._target_name_prefix()}-{qc_record_id(qc_record)}": [
f"{cls._target_name_prefix()}-{_get_qc_record_id(qc_record)}": [
(qc_record, molecule)
]
for qc_record, molecule in qc_records
Expand Down Expand Up @@ -275,9 +280,7 @@ def _generate_target(
assert len(qc_records) == 1
qc_record, off_molecule = qc_records[0]

qc_record_id = (
qc_record.extras["id"] if "id" in qc_record.extras else qc_record.id
)
qc_record_id = _get_qc_record_id(qc_record)

# form a Molecule object from the first torsion grid data
if isinstance(qc_record, TorsiondriveRecord):
Expand Down Expand Up @@ -472,9 +475,7 @@ def _create_vdata_file(
off_molecule: An OpenFF representation of the QC molecule.
"""

qc_record_id = (
qc_record.extras["id"] if "id" in qc_record.extras else qc_record.id
)
qc_record_id = _get_qc_record_id(qc_record)

try:
# this is a qcportal.record_models.BaseRecord
Expand Down Expand Up @@ -547,9 +548,7 @@ def _generate_target(

assert len(qc_records) == 1
qc_record, off_molecule = qc_records[0]
qc_record_id = (
qc_record.extras["id"] if "id" in qc_record.extras else qc_record.id
)
qc_record_id = _get_qc_record_id(qc_record)

fb_molecule = FBMolecule()
fb_molecule.Data = {
Expand Down Expand Up @@ -608,9 +607,7 @@ def _generate_target(
record_names = []

for i, (qc_record, off_molecule) in enumerate(qc_records):
qc_record_id = (
qc_record.extras["id"] if "id" in qc_record.extras else qc_record.id
)
qc_record_id = _get_qc_record_id(qc_record)

record_name = f"{qc_record_id}-{i}"
record_names.append(record_name)
Expand Down

0 comments on commit 8c4bad6

Please sign in to comment.