Reaction label is not validated when reactants/products are given, causing a late IndexError in flip_reaction

オープン 初心者向け
  • 何か問題がありますか？
#1,043 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 2 日以内に返信

まだ誰も着手していません。

評価

難易度
2/5
見積もり時間
1〜3時間
初心者へのやさしさ
76/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
python
領域
backend

調査の方向性

arc/reaction/reaction.py から始め、set_label_reactants_products と flip_reaction に注目してから、issue の YAML 例を再現します。必要な矢印を含まない指定されたラベルが、プロジェクトのセットアップやファミリーの判定より前に、既存の ReactionError メッセージとともに入力解析中に拒否されるようにしてください。

索引モデルが issue の本文から書いたものです。

説明

Describe the bug

If a reaction is given both an explicit label and explicit reactants/products, the label is never validated. ARC accepts the input, starts the project, and fails later inside family determination with a bare IndexError that says nothing about labels:

File "arc/reaction/reaction.py", line 582, in determine_family
    product_dicts = self.product_dicts
File "arc/family/family.py", line 525, in get_reaction_family_products
    flipped_rxn = rxn.flip_reaction(report_family=False)
File "arc/reaction/reaction.py", line 389, in flip_reaction
    reaction_dict['label'] = self.arrow.join([label_splits[1], label_splits[0]])
IndexError: list index out of range

The check already exists — it is just on the other branch. In set_label_reactants_products:

if not len(self.reactants) or not len(self.products):
    if self.label:
        if self.arrow not in self.label:
            raise ReactionError(f'A reaction label must contain an arrow ("{self.arrow}")')
        ...

That guard only runs when the reactants and products are absent. When they are supplied, the whole block is skipped, and the later

if not self.label:
    self.label = self.arrow.join(...)

does not fire either, because a label is present. The label therefore survives unvalidated until flip_reaction splits on the arrow and indexes [1].

How to reproduce

project: demo
species:
  - label: R
    smiles: '[CH2]CC'
  - label: P
    smiles: 'C[CH]C'
reactions:
  - label: my_reaction          # no arrow, and reactants/products are given below
    reactants: [R]
    products: [P]

ARC starts normally, prints the levels of theory, and then raises the IndexError above. With the label line removed it works, because ARC builds the label itself.

Expected behavior

First of all, we should decide if we allow users to not use <=> in the reaction label. If we do not, we would ideally want the fail at input parsing with the message that already exists, rather than partway through the run. Two options, either of which is a small change:

  1. Move the arrow check out of the if not len(self.reactants) or not len(self.products) branch so it applies to any user-supplied label.
  2. Have flip_reaction rebuild the label from r_species/p_species instead of string-splitting the existing one. That is more robust anyway, since the label is cosmetic once the species are known.

The reason this is worth more than a nicer traceback is timing: on a queueing cluster the failure surfaces after the project directory is created and the first jobs are being prepared, so the user finds out minutes later and from a message that does not name the cause. A label is exactly the kind of thing worth rejecting at parse time.

Installation information

  • ARC main @ a24e13b0
  • Python 3.14, Linux, PBS
主要言語
Python
スター
52
フォーク
25
平均マージ
1日 5時間
マージ済み PR（30日）
2

環境構築

  • Dockerfile または Docker Compose ファイルあり
  • プルリクエストのテンプレートなし
  • コントリビューションガイドなし

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

ReactionMechanismGenerator/ARC のほかの issue

ReactionMechanismGenerator/ARC の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。