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

Add Python 3 support #3

Merged
merged 11 commits into from
Nov 22, 2018
Merged

Add Python 3 support #3

merged 11 commits into from
Nov 22, 2018

Conversation

chiwanpark
Copy link
Contributor

@chiwanpark chiwanpark commented Nov 10, 2018

Python 3 지원을 위해 아래 사항을 수정했습니다.

  • six 라이브러리 추가 (py2/3 호환용, MIT license)
  • 코드가 py3에서도 동작하도록 수정
  • y_vocab.cPickle 파일 다시 만들어야하는 부분 README에 추가

기타 수정 사항

  • config.json의 오타 수정 (preidct -> predict)
  • Python 3.5에서만 발생하는 버그로 인해 map_async 결과 timeout 시간 감소

@ummae
Copy link
Contributor

ummae commented Nov 10, 2018

@chiwanpark 👍
가능하다면 y_vocab.cPickle 생성한 파일도 이름이 겹치지 않도록 해서 같이 포함하면 좋을 것 같아요.

config.json Outdated Show resolved Hide resolved
@chiwanpark
Copy link
Contributor Author

@ummae Python 3에서 사용할 y_vocab.py3.cPickle 파일 추가했습니다. 코드에서도 자동으로 Python 3인 경우 해당 파일을 불러오도록 수정했어요.

@GzuPark
Copy link
Contributor

GzuPark commented Nov 11, 2018

@chiwanpark 👏
README.md의 실행방법 4. 에서도 안내를 해주는게 좋지 않을까요?
예를 들면,

python2 evaluate.py evaluate predict.tsv ./data/train/data.h5py dev ./data/y_vocab.cPickle
python3 evaluate.py evaluate predict.tsv ./data/train/data.h5py dev ./data/y_vocab.py3.cPickle

Copy link
Contributor

@ummae ummae left a comment

Choose a reason for hiding this comment

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

data.py Show resolved Hide resolved
@chiwanpark
Copy link
Contributor Author

위 코드로 학습시킨 모델로 dev 계산하여 채점하면 아래와 같은 점수를 얻을 수 있습니다.

  • Python 2: 0.851424
  • Python 3: 0.845507

버전에 따른 점수 차이는 Python 버전 별 해시 함수 차이(built-in hash vs mmh3)로 생각됩니다.

@ummae
Copy link
Contributor

ummae commented Nov 22, 2018

👍

@ummae ummae merged commit b643e20 into kakao-arena:master Nov 22, 2018
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.

3 participants