Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

poetry の導入 #151

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

poetry の導入 #151

wants to merge 6 commits into from

Conversation

shmokmt
Copy link

@shmokmt shmokmt commented May 24, 2020

** Description 説明 **
通りすがりの岡本と申します。機械学習に知見のない自分でも貢献できそうなpoeryのissueがありましたので、PRを差し上げます。

  • poetry をプロジェクトに導入しました。
  • プロダクションコードから必要なサードパーティのライブラリを洗い出して追加しました。

Close #126

** Type of change 変更の種類**

Please delete options that are not relevant for your post.
関係のないオプションは削除してください

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Copy link
Author

@shmokmt shmokmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

補足コメントを追加しました。

pyproject.toml Outdated
@@ -0,0 +1,19 @@
[tool.poetry]
name = "somf"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

実際にpypiにpublishする際はsomfディレクトリを読みに行くと思うので、その際は
#84 をクローズする必要があるかと思います。

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いまここで package nameを somf にしちゃうと、poetry isntall 時にパッケージがインストールされなくなり
/tests/tutorials/ からのlibs import が全て失敗してしまうので、libs に変更します。
追々、パッケージ名を整えるフェーズに入ったら変えてもらえればと思います 🙏

pyproject.toml Outdated
Comment on lines 8 to 13
python = "^3.8"
numpy = "^1.18.4"
tensorflow = "^2.2.0"
tqdm = "^4.46.0"
sklearn = "^0.0"
scipy = "^1.4.1"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runtimeは以下のバージョンで導入しました。
不都合がある場合は教えて下さい。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

どのバージョンなら動くかとかあまり詳しく調べていないのですが,とりあえず自分の環境のバージョンを入れさせてもらいました!こうやって書くんだなと勉強になりました.ありがとうございます!

@shmokmt
Copy link
Author

shmokmt commented May 24, 2020

@ae14watanabe レビューをお願いいたします 🙏

@ae14watanabe
Copy link
Member

@shmokmt ありがとうございます!研究室外からのプルリクエストは初なので本当にありがたいです!後ほどレビューさせて頂きます!

@ae14watanabe ae14watanabe self-requested a review May 29, 2020 08:32
@shmokmt
Copy link
Author

shmokmt commented Jun 17, 2020

@ae14watanabe こちらレビュー状況いかがでしょうか? 👀

@ae14watanabe
Copy link
Member

お待たせしてすみません!やっと自分のローカル環境でpoetryが使えるようになったので今週中にreviewしたいと思います〜!

Copy link
Member

@ae14watanabe ae14watanabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自分の環境のバージョンをsuggestionしましたので反映してもらえると助かります!

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated
Comment on lines 8 to 13
python = "^3.8"
numpy = "^1.18.4"
tensorflow = "^2.2.0"
tqdm = "^4.46.0"
sklearn = "^0.0"
scipy = "^1.4.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

どのバージョンなら動くかとかあまり詳しく調べていないのですが,とりあえず自分の環境のバージョンを入れさせてもらいました!こうやって書くんだなと勉強になりました.ありがとうございます!

pyproject.toml Show resolved Hide resolved
Copy link
Author

@shmokmt shmokmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pyproject.tomlを反映した後にlockファイルを生成する必要があり、suggestionをコミットするだけでマージできそうにないので、一旦ローカルで指摘いただいた箇所を反映します。

pyproject.toml Outdated
@@ -0,0 +1,19 @@
[tool.poetry]
name = "somf"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いまここで package nameを somf にしちゃうと、poetry isntall 時にパッケージがインストールされなくなり
/tests/tutorials/ からのlibs import が全て失敗してしまうので、libs に変更します。
追々、パッケージ名を整えるフェーズに入ったら変えてもらえればと思います 🙏

@ae14watanabe
Copy link
Member

承知しました!ありがとうございます〜泣

@shmokmt
Copy link
Author

shmokmt commented Jun 21, 2020

動作確認のために1ファイルだけテストコード走らせました。

.venv ❯ python tests/som/allclose_somkl/allclose_somklishida_vs_somkl_watanabe.py
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 200/200 [00:00<00:00, 1752.80it/s]
100%|█████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 200/200 [00:00<00:00, 1609.32it/s]
.
----------------------------------------------------------------------
Ran 1 test in 0.251s

OK

@shmokmt
Copy link
Author

shmokmt commented Jun 21, 2020

@ae14watanabe 指摘いただいた箇所を修正しました。もう1度レビューお願いします 🙏

@ae14watanabe
Copy link
Member

@shmokmt ありがとうございます!現在自分がちょっとpoetry周りでトラブルが起きておりまして,レビューもうしばらくお待ちください!

@ae14watanabe
Copy link
Member

@shmokmt お久しぶりです!対応遅くなっていて大変申し訳ないのですが,ブランチがこちらから見えなくなっていまして,これってどういう原因が考えられるのでしょうか…?汗
image

@shmokmt
Copy link
Author

shmokmt commented Jul 16, 2020

@ae14watanabe
すみません、先日forkしたリポジトリを私が誤って消してしまったのが原因のようです。
PRを後日作り直そうと思います 💦

@ae14watanabe
Copy link
Member

あらそうでしたか汗
お手数おかけします

@TetraMiyazaki
Copy link
Contributor

すみません.今更確認してます.unknown repositoryってなってるの初めてみました.file changesは残っているのでどうにかならないのかな

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

poetryの導入
3 participants