Convert mutable type which is unhasable to immutable types #123

kanghyojun · 2017-03-19T14:58:05Z

it fixes nirum-lang/nirum-python#49. this PR propose convert a type that could be mutable to immutable type always rather than raise error on runtime.

dahlia

함수 이름에 대한 댓글을 많이 달았습니다. toX와 같은 이름들이 쓰였는데, 하스켈의 X로 만든다는 느낌이 먼저 들었기 때문입니다. 실제로는 파이썬의 값을 파이썬의 X로 만드는 파이썬 코드를 생성해주는 함수였기 때문에 다른 이름들을 제안했습니다.

dahlia · 2017-03-19T15:25:29Z

src/Nirum/Targets/Python.hs

+    attributeValue :: T.Text
+    attributeValue = case fieldType' of
+                         SetModifier _ -> [qq|frozenset($attributeName)|]
+                         ListModifier _ -> [qq|tuple($attributeName)|]


리스트를 튜플로 표현하는 것이 적절하지는 않은 것 같습니다. 튜플을 불변 리스트로 보는 사람이 별로 없기 때문입니다. 차라리 런타임에 ImmutableList를 만들어서 넣고 그걸 쓰는 건 어떨까요?

dahlia · 2017-03-19T15:27:31Z

src/Nirum/Targets/Python.hs

+    fieldName' :: Name
+    fieldName' = fieldName field
+    fieldType' :: TypeExpression
+    fieldType' = TD.fieldType field


fieldName', fieldType' 이 둘은 getter 쓰는 것보다 데이터 생성자 써서 패턴 매칭으로 구하는 게 더 깔끔하지 않을까요?

dahlia · 2017-03-19T15:29:22Z

src/Nirum/Targets/Python.hs

@@ -329,6 +339,28 @@ returnCompiler = do
                        Python2 -> ""
                        Python3 -> [qq| -> $r|]

+convertFieldToImmutable :: Field -> Code
+convertFieldToImmutable field =


compileFieldInitializer 정도의 이름을 제안합니다.

dahlia · 2017-03-19T15:30:48Z

src/Nirum/Targets/Python.hs

+                         _ -> attributeName
+
+toClassInitialValues :: [Field] -> [Code]
+toClassInitialValues fields =


비슷하게 compileClassFieldInitializers 정도의 이름을 제안합니다.

dahlia · 2017-03-19T15:35:16Z

src/Nirum/Targets/Python.hs

+toArgumentCode :: (ParameterName -> ParameterType -> Code)
+               -> [(T.Text, Code)]
+               -> Code
+toArgumentCode gen nameNTypes = toIndentedCodes (uncurry gen) nameNTypes ", "


Parameter는 보통 형식 인자(인자의 이름이나 타입과 같은 모양)을 뜻하고 argument는 그 인자에 들어가는 실제 인자값을 뜻합니다. 예를 들어 def f(a, b): 같은 함수를 정의할 때 a나 b는 parameters지만 그걸 호출하는 f(1, 2)에서 1이나 2는 arguments입니다.

그런 차원에서 생각하면 함수 이름은 argument라는 말보다는 parameter라는 말을 쓰는 게 나을 것 같네요.

compileParameters 정도의 이름을 제안합니다.

dahlia · 2017-03-19T15:36:37Z

src/Nirum/Targets/Python.hs

+toArgumentCode gen nameNTypes = toIndentedCodes (uncurry gen) nameNTypes ", "
+
+toInitialValueCode :: DS.DeclarationSet Field -> Code
+toInitialValueCode fields =


compileFieldInitializers 정도의 이름을 제안합니다. 그리고 이하의 convertFieldToImmutable, toClassInitialValues 같은 함수들은 단위 테스트를 붙이기 위한 것이 아니라면 차라리 이 함수의 where 절 아래로 넣는 게 낫지 않을까요?

kanghyojun · 2017-03-19T16:26:07Z

@dahlia ImmutableList로 갈아치우기위해 nirum-lang/nirum-python#65 를 올렸습니다. 물론 ImmutableList를 사용하기 위해선 [qq|ImmutableList(...)|]이런 식으로 바로 되는 것은 아니고, 런타임을 추가하기위해 thirdPartyImports를 호출해야합니다. 일단 런타임 머지, PyPI 배포 후에 이 PR은 다시 진행합니다.

dahlia · 2017-03-23T17:24:04Z

src/Nirum/Targets/Python.hs

+        case fieldType' of
+            SetModifier _ -> return [qq|frozenset($attributeName)|]
+            ListModifier _ -> do
+                let imports = [("nirum.datastructures", ["List"])]


It prevents Nirum types to be named as list since it can make name conflict. Seems nirum.datastructures need to export aliases e.g. list_type, map_type, etc.

i need to modify nirum-python first, then i'll fix this!

nirum-lang/nirum-python#70 nirum-python fixed. From now on, List aliased as list_type.

kanghyojun · 2017-03-31T15:48:52Z

🙇🏼 Kore o kakuninshitekudasai 🙇🏼

dahlia · 2017-04-01T08:28:37Z

test/python/typeconversion_test.py

+    album = Album(name='25', tracks=[
+        Song(name='Hello')
+    ])
+    assert isinstance(album.tracks, tuple)


tuple이 아니라 nirum.datastructures.List인지 검사해야 할 것 같습니다.

- Gather some functions in where - Rename convertFieldToImmutable -> compileFieldInitializer - Use pattern-matching to get value of field

- Update minimum required version of nirum - Unicode lietral have to be used on Python2.7

Importing "List" would raise error, because "List" could be named on user-define variable. so list_type which is alias for List is imported.

kanghyojun added typ:bug Type: Bug/defect typ:enhance Type: Enhancement/new feature labels Mar 19, 2017

kanghyojun self-assigned this Mar 19, 2017

kanghyojun requested review from dahlia, yjroot, Kroisse, AiOO and heejongahn March 19, 2017 14:58

kanghyojun mentioned this pull request Mar 19, 2017

Field values must be hashable nirum-lang/nirum-python#49

Closed

dahlia requested changes Mar 19, 2017

View reviewed changes

dahlia requested changes Mar 23, 2017

View reviewed changes

kanghyojun force-pushed the field-must-be-hashable branch from 5d61368 to 7ad23b8 Compare March 31, 2017 15:47

dahlia requested changes Apr 1, 2017

View reviewed changes

Hyojun Kang and others added 8 commits April 20, 2017 22:31

Convert mutable type to immutable type

206c004

Refactor repeated pattern

05094c6

Add testcase for type cast

6a9007f

Rename toArgumentCode -> compileParameters

c630623

Rename toIndentedCodes -> compileFieldInitializers

f665bde

- Gather some functions in where - Rename convertFieldToImmutable -> compileFieldInitializer - Use pattern-matching to get value of field

Import nirum datastructure in compileFieldInitializers

45b80b4

Test nirum list is appropriate type on python

cefdc51

wip

f7143e6

kanghyojun force-pushed the field-must-be-hashable branch from 7ad23b8 to f7143e6 Compare April 20, 2017 13:54

kanghyojun mentioned this pull request May 2, 2017

Use abstract class to compare with immutable type nirum-lang/nirum-python#73

Merged

kanghyojun added 3 commits May 2, 2017 23:08

Use nirum>=0.4.1

952a627

- Update minimum required version of nirum - Unicode lietral have to be used on Python2.7

Prevent names from overlapping

20aacce

Importing "List" would raise error, because "List" could be named on user-define variable. so list_type which is alias for List is imported.

Use list_type

01ad3d8

dahlia approved these changes Jun 1, 2017

View reviewed changes

kanghyojun merged commit 9e24232 into nirum-lang:master Jun 1, 2017

dahlia added cmp:compiler Component: Compiler backend (e.g., annotation processors, code generators) target:python labels Aug 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert mutable type which is unhasable to immutable types #123

Convert mutable type which is unhasable to immutable types #123

kanghyojun commented Mar 19, 2017

dahlia left a comment

dahlia Mar 19, 2017

dahlia Mar 19, 2017

dahlia Mar 19, 2017

dahlia Mar 19, 2017

dahlia Mar 19, 2017

dahlia Mar 19, 2017

kanghyojun commented Mar 19, 2017

dahlia Mar 23, 2017

kanghyojun Mar 31, 2017

kanghyojun Mar 31, 2017

kanghyojun commented Mar 31, 2017

dahlia Apr 1, 2017

Convert mutable type which is unhasable to immutable types #123

Convert mutable type which is unhasable to immutable types #123

Conversation

kanghyojun commented Mar 19, 2017

dahlia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanghyojun commented Mar 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanghyojun commented Mar 31, 2017

Choose a reason for hiding this comment