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

Remove description.py and change author name #194

Merged
merged 3 commits into from May 28, 2018

Conversation

Projects
None yet
3 participants
@e9t
Copy link
Member

e9t commented May 28, 2018

이 PR은 다음의 변경사항을 포함합니다:

  1. 패키지의 metadata가 description.py, konlpy/__init__.py에 양분되어 있어서 전자를 없애고 후자만 남겼습니다.
  2. __version__의 경우 koshort 🎉 의 distribution version을 가져오도록 되어 있어서 삭제했습니다.
  3. 이 때 pkg_resources를 통해 버전을 외부에서 import 해오는 것은 python setup.py install을 했을 때 현재 패키지의 버전이 아니라 기존 패키지 버전으로 설치되게 하는 것 같아서 삭제했는데요, 혹시 제가 틀렸다면 다시 수정 부탁드립니다.
  4. 기존에 패키지의 author name이 Lucy Park으로 되어 있었는데, Team KoNLPy로 수정했습니다. 제 authorship을 걷어내고 팀과 커뮤니티가 마음껏 참여하고 기여할 수 있는 프로젝트가 되었으면 합니다.

@e9t e9t requested a review from nyanye May 28, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-0.3%) to 56.842% when pulling 910c74f on e9t:fix-version into cf24bd8 on konlpy:master.

3 similar comments
@coveralls

This comment has been minimized.

Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-0.3%) to 56.842% when pulling 910c74f on e9t:fix-version into cf24bd8 on konlpy:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-0.3%) to 56.842% when pulling 910c74f on e9t:fix-version into cf24bd8 on konlpy:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage decreased (-0.3%) to 56.842% when pulling 910c74f on e9t:fix-version into cf24bd8 on konlpy:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 28, 2018

Coverage Status

Coverage increased (+0.1%) to 57.292% when pulling 5130e8e on e9t:fix-version into cf24bd8 on konlpy:master.

@nyanye
Copy link
Member

nyanye left a comment

from konlpy import __version__을 setup.py에서 실행할 경우 dependency가 설치되기 이전에 init.py의 동작이 실행되고 여기서 JPype를 요구하는 jvm이 사전 import되기 때문에 설치에 실패할 수 있습니다.
이를 방지할 수 있는 방법에는 init.py에서 pkg_resources를 이용하는 방법, setup.py에서 re.search를 이용하는 방법이 있습니다.

https://github.com/amueller/word_cloud/blob/master/setup.py

🐱

추가:

  • pkg_resources의 경우 venv에서 사용했을 때 버젼을 가져오지 못하는 문제가 생길 수 있어 좋지 않음!
@e9t

This comment has been minimized.

Copy link
Member

e9t commented May 28, 2018

그러네요!
리뷰 감사합니다.

pkg_resources의 경우 아래와 같은 caveat이 있어서 spacy의 메타데이터 방식을 참고해서 수정해보았습니다.

Be aware that the pkg_resources API only knows about what’s in the installation metadata, which is not necessarily the code that’s currently imported.

@nyanye nyanye merged commit eb9dc53 into konlpy:master May 28, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
coverage/coveralls Coverage increased (+0.1%) to 57.292%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment