-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support Python2 targets!! #85
Conversation
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python 3에서는 __eq__
만 구현해도 __ne__
가 같이 구현되는데, Python 2는 __ne__
도 직접 구현해줘야 합니다.
#!/usr/bin/env bash | ||
|
||
stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture | ||
pushd "nirum_fixture"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
세미콜론 필요 없습니다. (기존에 .travis.yml 파일에서 세미콜론을 쓰는 이유는 여러줄로 써도 Travis CI가 한 줄로 합쳐버리기 때문…)
stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture | ||
pushd "nirum_fixture"; | ||
pip install -e . | ||
popd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 마찬가지로 세미콜론 필요 없습니다.
@@ -0,0 +1,8 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set -e
옵션 줍시다.
@@ -0,0 +1,8 @@ | |||
#!/usr/bin/env bash | |||
|
|||
stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&&
연산자로 잇지 않고 이 스크립트 자체에 set -e
옵션을 주고 두 줄로 고치는 게 나을 듯
- ./lint.sh | ||
- ./pythontest.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앞으로 다른 타겟 테스트까지 한번에 다 돌리는 스크립트로 만드는 게 나을 듯합니다. 그러려먼 파일명도 test.sh 정도로 줄이는 게 낫겠네요.
@@ -210,6 +233,21 @@ toIndentedCodes :: (a -> T.Text) -> [a] -> T.Text -> T.Text | |||
toIndentedCodes f traversable concatenator = | |||
T.intercalate concatenator $ map f traversable | |||
|
|||
quote :: T.Text -> T.Text | |||
quote s = '\'' `T.cons` s `T.snoc` '\'' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그냥 [qq|'{s}'|]
로 쓰면 더 깔끔하지 않을까요?
def __repr__(self) -> str: | ||
return '\{0.__module__\}.\{0.__qualname__\}(\{1\})'.format( | ||
def __repr__(self){ ret "str" }: | ||
return '\{0.__module__\}.\{1\}(\{2\})'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typing._type_repr()
가 모듈 이름까지 포함해서 반환하는 걸로 알고 있습니다. 0.__module__
을 덧붙이면 안될 겁니다.
- Use `set -e` - Remove useless semicolon - Don't use `&&`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test/python/ 아래쪽 파일도 flake8 등을 돌립시다.
@@ -35,7 +35,7 @@ module Nirum.Targets.Python ( Code | |||
, unionInstallRequires | |||
) where | |||
|
|||
import Control.Monad.State (modify) | |||
import Control.Monad.State as ST (get, modify) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qualified
씁시다.
python3SourceDirectory :: T.Text | ||
python3SourceDirectory = "src" | ||
python2SourceDirectory :: T.Text | ||
python2SourceDirectory = "src-py2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
python2SourceDirectory = "src-py2" | ||
|
||
emptyContext :: PythonVersion -> CodeGenContext | ||
emptyContext pythonVersion' = CodeGenContext { standardImports = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수명 empty
로 줄입시다.
typeHelpers' :: PythonVersion | ||
-> (T.Text -> T.Text -> T.Text, T.Text -> T.Text) | ||
typeHelpers' Python2 = (\n _ -> n, \_ -> "") | ||
typeHelpers' Python3 = (\n t -> [qq|$n: $t|], \t -> [qq| -> $t|]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typeHelpers :: CodeGen (T.Text -> T.Text -> T.Text, T.Text -> T.Text)
typeHelpers = do
ver <- getPythonVersion
return $ case ver of
Python2 -> ...
return '\{0.__module__\}.\{0.__qualname__\}(\{1\})'.format( | ||
type(self), | ||
def __repr__(self){ ret "str" }: | ||
return '\{0\}(\{2\})'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀 __repr__
도 테스트하죠
|
||
def __repr__(self) -> str: | ||
def __repr__(self){ ret "str" }: | ||
return '\{0.__module__\}.\{0.__qualname__\}(\{1!r\})'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
return '\{0.__module__\}.\{0.__qualname__\}(\{1\})'.format( | ||
type(self), | ||
def __repr__(self){ret "bool"}: | ||
return '\{0\}(\{1\})'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트합시다.
return isinstance(other, $className) and all( | ||
getattr(self, attr) == getattr(other, attr) | ||
for attr in self.__slots__ | ||
) | ||
|
||
def __nirum_serialize__(self) -> typing.Mapping[str, typing.Any]: | ||
def __ne__(self, other){ ret "bool" }: | ||
return not self.__eq__(other) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
|
||
SOURCE_ROOT = '{python3SourceDirectory}' | ||
|
||
if sys.version_info[0] == 2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if sys.version_info < (3,0):
이렇게 비교하는 게 나을 듯
@@ -916,6 +576,7 @@ spec = parallel $ do | |||
(req4 `unionInstallRequires` req5) `shouldBe` req6 | |||
(req5 `unionInstallRequires` req4) `shouldBe` req6 | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👀
six | ||
flake8 | ||
nirum | ||
pytest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 git+git://github.com/spoqa/nirum.git@3.0.1
추가합시다.
5ff29ef
to
6ffa991
Compare
c6ac075
to
3c118b5
Compare
3bffd62
to
08a199f
Compare
ac42ed4
to
a6e7346
Compare
#50
@dahlia @Kroisse 이런식으로 진행하면될까요?.. 근데 겹치는 로직(type annotation만 없으면된다던지)은 어떻게처리해야할지 감이안오네요. 대충 머리속에선 이런걸 그렸는데...
#50 해결
nirum_fixture
를 밖으로 빼서 정리하고 원래scanPacakage
테스트 케이스 살리기tox
적용