Skip to content

Support Python2 targets!!#85

Merged
kanghyojun merged 29 commits into
nirum-lang:masterfrom
kanghyojun:python2-target
Nov 24, 2016
Merged

Support Python2 targets!!#85
kanghyojun merged 29 commits into
nirum-lang:masterfrom
kanghyojun:python2-target

Conversation

@kanghyojun

@kanghyojun kanghyojun commented Oct 23, 2016

Copy link
Copy Markdown
Member

#50

@dahlia @Kroisse 이런식으로 진행하면될까요?.. 근데 겹치는 로직(type annotation만 없으면된다던지)은 어떻게처리해야할지 감이안오네요. 대충 머리속에선 이런걸 그렸는데...

pythonMethod PythonVersion T.Text PythonArgument Code
pythonMethod Python2 name arguments body returnType = [qq|
    def $name($arguments) # return $returnType:
        $body
|]
pythonMethod Python3 name arguments Code returnType = [qq|
    def $name($arguments) -> $returnType:
        $body
|]

compileTypeExpression pythonVersion RecordType ... =
   ....
   {pythonMethod pythonVersion "hash" .... .... PythonStr}
   ....

#50 해결

  • 리뷰 받은 사항 남은거 정리
  • nirum_fixture 를 밖으로 빼서 정리하고 원래 scanPacakage 테스트 케이스 살리기
  • tox 적용

@kanghyojun kanghyojun added the typ:enhance Type: Enhancement/new feature label Oct 23, 2016
@kanghyojun

kanghyojun commented Oct 25, 2016

Copy link
Copy Markdown
Member Author
  • 테스트 통과시키기
  • python2 테스트하면서 쉘스크립트로 옮겨서 pytest가 테스트하게하기

@kanghyojun

kanghyojun commented Nov 3, 2016

Copy link
Copy Markdown
Member Author
  • stack test 통과
  • python3 에서 annotation 이상하게보이는것 수정

@dahlia dahlia left a comment

Copy link
Copy Markdown
Member

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__도 직접 구현해줘야 합니다.

Comment thread pythontest.sh Outdated
#!/usr/bin/env bash

stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture
pushd "nirum_fixture";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

세미콜론 필요 없습니다. (기존에 .travis.yml 파일에서 세미콜론을 쓰는 이유는 여러줄로 써도 Travis CI가 한 줄로 합쳐버리기 때문…)

Comment thread pythontest.sh Outdated
stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture
pushd "nirum_fixture";
pip install -e .
popd;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

여기도 마찬가지로 세미콜론 필요 없습니다.

Comment thread pythontest.sh Outdated
@@ -0,0 +1,8 @@
#!/usr/bin/env bash

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

set -e 옵션 줍시다.

Comment thread pythontest.sh Outdated
@@ -0,0 +1,8 @@
#!/usr/bin/env bash

stack build && stack exec -- nirum -o nirum_fixture test/nirum_fixture

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

&& 연산자로 잇지 않고 이 스크립트 자체에 set -e 옵션을 주고 두 줄로 고치는 게 나을 듯

Comment thread .travis.yml Outdated
- pip install test/python/requirements.txt
- stack --no-terminal test
- ./lint.sh
- ./pythontest.sh

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

앞으로 다른 타겟 테스트까지 한번에 다 돌리는 스크립트로 만드는 게 나을 듯합니다. 그러려먼 파일명도 test.sh 정도로 줄이는 게 낫겠네요.

Comment thread src/Nirum/Targets/Python.hs Outdated
T.intercalate concatenator $ map f traversable

quote :: T.Text -> T.Text
quote s = '\'' `T.cons` s `T.snoc` '\''

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

그냥 [qq|'{s}'|]로 쓰면 더 깔끔하지 않을까요?

Comment thread src/Nirum/Targets/Python.hs Outdated
def __repr__(self) -> str:
return '\{0.__module__\}.\{0.__qualname__\}(\{1\})'.format(
def __repr__(self){ ret "str" }:
return '\{0.__module__\}.\{1\}(\{2\})'.format(

Copy link
Copy Markdown
Member

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__을 덧붙이면 안될 겁니다.

@kanghyojun kanghyojun changed the title [WIP] Python2 targets Support Python2 targets!! Nov 14, 2016

@dahlia dahlia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

test/python/ 아래쪽 파일도 flake8 등을 돌립시다.

Comment thread src/Nirum/Targets/Python.hs Outdated
) where

import Control.Monad.State (modify)
import Control.Monad.State as ST (get, modify)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

qualified 씁시다.

Comment thread src/Nirum/Targets/Python.hs Outdated
python3SourceDirectory :: T.Text
python3SourceDirectory = "src"
python2SourceDirectory :: T.Text
python2SourceDirectory = "src-py2"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👀

Comment thread src/Nirum/Targets/Python.hs Outdated
python2SourceDirectory = "src-py2"

emptyContext :: PythonVersion -> CodeGenContext
emptyContext pythonVersion' = CodeGenContext { standardImports = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

함수명 empty로 줄입시다.

Comment thread src/Nirum/Targets/Python.hs Outdated
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|])

Copy link
Copy Markdown
Member

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 -> ...

Comment thread src/Nirum/Targets/Python.hs Outdated
return '\{0.__module__\}.\{0.__qualname__\}(\{1\})'.format(
type(self),
def __repr__(self){ ret "str" }:
return '\{0\}(\{2\})'.format(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👀 __repr__도 테스트하죠

Comment thread src/Nirum/Targets/Python.hs Outdated

def __repr__(self) -> str:
def __repr__(self){ ret "str" }:
return '\{0.__module__\}.\{0.__qualname__\}(\{1!r\})'.format(

Copy link
Copy Markdown
Member

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(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

테스트합시다.

Comment thread src/Nirum/Targets/Python.hs Outdated

def __nirum_serialize__(self) -> typing.Mapping[str, typing.Any]:
def __ne__(self, other){ ret "bool" }:
return not self.__eq__(other)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👀

Comment thread src/Nirum/Targets/Python.hs Outdated

SOURCE_ROOT = '{python3SourceDirectory}'

if sys.version_info[0] == 2:

Copy link
Copy Markdown
Member

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): 이렇게 비교하는 게 나을 듯

Comment thread test/Nirum/Targets/PythonSpec.hs Outdated
(req4 `unionInstallRequires` req5) `shouldBe` req6
(req5 `unionInstallRequires` req4) `shouldBe` req6


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👀

Comment thread tox.ini
six
flake8
nirum
pytest

Copy link
Copy Markdown
Member

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 추가합시다.

@kanghyojun
kanghyojun force-pushed the python2-target branch 2 times, most recently from 3bffd62 to 08a199f Compare November 23, 2016 15:14
@kanghyojun
kanghyojun merged commit 2527e07 into nirum-lang:master Nov 24, 2016
@kanghyojun kanghyojun mentioned this pull request Nov 24, 2016
@dahlia dahlia added cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) target:python labels Aug 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) target:python typ:enhance Type: Enhancement/new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants