Add help command - Githubissues

aki2274 commented 3 months ago

argparseを使って引数を受け取るようにしました． createdataをサブコマンド化するための動作も行っています．このあたりの引数の受け取り方は正しいでしょうか．ご確認いただければと思います．また，例外処理の部分も併せてここで解決できればと思います．コメントよろしくお願いします．

akikuno commented 3 months ago

@aki2274

例外処理の部分も併せてここで解決できればと思います

について、具体的にどの部分を見るべきか、ご教授いただけますか？🙇

akikuno commented 3 months ago

related to #90

aki2274 commented 3 months ago

サブコマンド名がcreateですが、こちらpreprocessとか、前処理であることを明示したほうが良いかも、と思いました👀．transcript nameはtranscript name(Refseq ID)と明記するとよいでしょう

承知しました．そのように変更します．

関連して、こちら一度preprocessを行ったら、同一の入力が来た場合にはスキップするべき（もう一度実行する必要がないので）と思いますが、そのようなスキップ処理は実装されていますか？

スキップ処理は行っていないです．確認して行うようにします．

chromosome_fasta_pathのヘルプメッセージの"Path to the chromosome fasta file.(ex. mm39.fa)"について、ex.ではなくe.g. でしょうか？

e.gのほうが一般的なようですね．修正します．

efflat_pathやchromosome_fasta_pathが、正しく渡されているか、それぞれの形式を判定する関数はございましたか？

こちらの最後の関数で行っているつもりです．またargparsreでpathを受け取るようになっています．この手法でよろしいですか？

たとえば、refflat_pathに、全然違うファイルを誤って入力してしまった場合にエラーは出ますか？

1つ上のような対応なのですが不十分ですかね？

aki2274 commented 3 months ago

例外処理の部分も併せてここで解決できればと思います

main.py,create_gene_dataclass.py,main.py の3つになります．（ブランチが違ったらすいません．） printとraiseが十分そうであるか確認いただければと思います．

akikuno commented 3 months ago

efflat_pathやchromosome_fasta_pathが、正しく渡されているか、それぞれの形式を判定する関数はございましたか？

のご対応について、ありがとうございます。

拡張子で判断するのはとても良い対応だと思います。

一点、The fasta file must be .fa fileですが、.fastaを加えても良いかと思います。ちなみに、DAJIN2ではfa, fasta, fa.gz, fasta.gzの４つをサポートしています。

fastaファイルは主に以下の拡張子で扱われます。 <一般的>

.fa
.fasta

<たまに gz圧縮>

.fa.gz
.fasta.gz

<極稀なので無視してOK （DAJIN2では無視してます。issueが来たら対応しようかと😅）>

.mfa
.mfasta

aki2274 commented 3 months ago

.fastaも受け付けるようにしました．とりあえず圧縮ファイルの解凍は各人でお願いする感じでもよいですかね...

akikuno commented 3 months ago

printとraiseが十分そうであるか確認いただければと思います．

拝読しました！

https://github.com/aki2274/KOnezumi-AID/blob/c72d22f14cbe197894ffc239d675a9e008a023ef/src/konezumiaid/main.py#L127

↑ raiseではエラー終了となってしまうので、正常終了(printやlogging.info)のほうが適切です。

https://github.com/aki2274/KOnezumi-AID/blob/3e8997ae2e2e223a17e438a58752493a6b4f0f18/src/konezumiaid/create_gene_dataclass.py#L36

↑ 利用者はtranscript_seq_dictが何なのかがわからないので、明示的にしたほうが良いでしょう

Transcript sequence doese not find your dataset ({refFlat.txt} and {mm39.fa}).みたいにできますか？

aki2274 commented 3 months ago

raiseではエラー終了となってしまうので、正常終了(printやlogging.info)のほうが適切です。

printでエラー文を示し，sys,exit()で終了するようにしました．データセットの作成, KOnezumi-AIDのmain エラーが起こった場合の挙動を変更しました．こちらについてはsysではなくNoneを返すようにしました．意図としては，複数転写産物のうち1つだけレコードされていないといった異常があった場合はコードを終了するのではなくその転写産物をスキップするのが良いと感じたからです．(KOnezumi-AID的にはそのような状況は起こらないと思いますが．)

Transcript sequence doese not find your dataset ({refFlat.txt} and {mm39.fa}).みたいにできますか？

エラーが起こった場合の挙動を変更しました．こちらで文章を変えています．引数にファイルパスを受け付けるのはpreprocessの時のみなのでこのような表示にしました．

akikuno commented 3 months ago

すみません、言葉足らずでした…！

raiseではエラー終了となってしまうので、正常終了(printやlogging.info)のほうが適切です。

こちらは、以下のコードだけに該当する助言でした。

https://github.com/aki2274/KOnezumi-AID/blob/c72d22f14cbe197894ffc239d675a9e008a023ef/src/konezumiaid/main.py#L127

こちらでは、「すでに前処理が終了しています」ということなので、エラーではなく正常終了にしたほうが良いのですが、他の箇所はなんのエラーなのかを明示するために、raiseを使うのが適切です。。。

print/sys.exit(1)にご変更していただいたのは大変申し訳無いのですが、konezumiaid.main以外のエラーコードを、raiseに戻していただけますと幸いです🙇

ご不明なことがございましたら何なりとご指摘ください。

aki2274 commented 3 months ago

勘違いしていました．raiseに戻しておきました．

エラーが起こった場合の挙動を変更しました．こちらで文章を変えています．引数にファイルパスを受け付けるのはpreprocessの時のみなのでこのような表示にしました．

こちらはそのままでよろしいでしょうか．

akikuno commented 3 months ago

早速のご対応をありがとうございます。

エラーが起こった場合の挙動を変更しました．こちらで文章を変えています．引数にファイルパスを受け付けるのはpreprocessの時のみなのでこのような表示にしました．

文章の変更はとても良いと思います！

一方で、私の理解が追いついていないのですが、エラー扱いとしない理由はなんでしょうか？

両者とも、printではなくraiseのほうが適切に思えるのですが👀

aki2274 commented 3 months ago

まず，

(KOnezumi-AID的にはそのような状況は起こらないと思いますが．)

こちらはgene symbolを受け付けてそのtranscript ごとにKOnezumiAIDを動かしているようなながれになっています． raiseで終了するとgene symbolでの検索自体にエラーを出します．しかし，その中のtranscriptの1つがエラーでもほかのtranscriptが動くのならばそれを表示したほうが良いのでないかと思ってやりました．refFlatのデータと配列情報が別にあるので万が一の参照ミスがある場合の対応かなと考えていました． gene Xに3つのtranscriptsがあった時に，1, 〇 2, RefSeq idから正しく配列を取得できない 3, 〇の場合1,3の共通するガイドをgene Xの候補として取り出すイメージです．説明下手ですいません，伝わりますか？ですが，書いててraiseのほうがよい気もしました．意図が伝わってましたらどちらのほうがよいか意見いただければと思います．

akikuno commented 3 months ago

ご説明をいただき、ありがとうございます！

raiseで終了するとgene symbolでの検索自体にエラーを出します．しかし，その中のtranscriptの1つがエラーでもほかのtranscriptが動くのならばそれを表示したほうが良いのでないかと思ってやりました．

理解しました🙇 滝さんのご指摘の通り、このような状況は（前処理でrefflat.txtとmm39.faを正しく利用している場合には）には起こらないと思いますが、いかがでしょうか。

もしtranscriptが動かない場合には、前処理に使った「refFlatのデータと配列情報」が間違っている可能性が高い、という印象があります。前処理に間違ったデータを使用したケース以外で、このエラーを起こす可能性はありますか？前処理に間違ったデータを使用したケースでは、他のgRNAのレポートが正しい保証はないので、raiseでエラーを出したほうが良いかと存じます。

aki2274 commented 3 months ago

このような状況は（前処理でrefflat.txtとmm39.faを正しく利用している場合には）には起こらないと思いますが

同感です．こちらでraiseに修正させていただきました．ブランチをまたいでしまって申し訳ないです．

aki2274 / KOnezumi-AID

Add help command #92