Skip to content

Conversation

@khun-api
Copy link

@khun-api khun-api commented Oct 9, 2022

Repository : our repository link
Visualize : visualize notebook
Json : Json

Team member

@annibuliful
Copy link
Collaborator

@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

ตัว https://github.com/devmountaintechfest/hackathon-season2/pull/18/files#diff-525565c956f875698bc74ac798b474aa561f4570464b770434888cfbf34bde48R1 ไม่ใช่ standard library ครับ

หมายถึงยังไงครับ @annibuliful ทำไมถึง invalid ครับ

@annibuliful
Copy link
Collaborator

ตัว https://github.com/devmountaintechfest/hackathon-season2/pull/18/files#diff-525565c956f875698bc74ac798b474aa561f4570464b770434888cfbf34bde48R1 ไม่ใช่ standard library ครับ

หมายถึงยังไงครับ @annibuliful ทำไมถึง invalid ครับ

https://lxml.de/ ตัวนี้เป็น library ที่ต้อง install ถึงจะใช้งานได้ซึ่งผิดกฎที่เราได้แจ้งไว้ใน live ครับ
Screen Shot 2565-10-09 at 21 15 08

@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

ตัว https://github.com/devmountaintechfest/hackathon-season2/pull/18/files#diff-525565c956f875698bc74ac798b474aa561f4570464b770434888cfbf34bde48R1 ไม่ใช่ standard library ครับ

หมายถึงยังไงครับ @annibuliful ทำไมถึง invalid ครับ

https://lxml.de/ ตัวนี้เป็น library ที่ต้อง install ถึงจะใช้งานได้ซึ่งผิดกฎที่เราได้แจ้งไว้ใน live ครับ Screen Shot 2565-10-09 at 21 15 08

Lib ตัวนี้ใช้​ read data ส่วน​ ขั้นตอน​ transform เป็น​ logic นะครับ​ @annibuliful ที่เขียนไว้คือ​ convert to json. ไม่ใช่เหรอครับ​ แล้วมีเวลาให้แก้ถึงกี่โมงครับ​และอีกอย่างก็ไม่ได้เขียน​ห้าม​ลง​ package อื่นๆนอกจาก​ standard

@annibuliful
Copy link
Collaborator

  1. มีการลง library เยอะมากเลย
  2. ถ้าจะเอาไว้ read data ทำไมถึงไม่เลือกใช้​ https://docs.python.org/3/library/xml.etree.elementtree.html แทนครับ?

@annibuliful
Copy link
Collaborator

https://lxml.de/

ตัวนี้ไม่ใช่ standard library นะครับ

@annibuliful
Copy link
Collaborator

ส่วนเรื่องเวลาได้แจ้งชัดเจนตั้งแต่ในไลฟ์และมีการโพสซ้ำไปแล้ว
Screen Shot 2565-10-09 at 21 27 24

@annibuliful
Copy link
Collaborator

และโค๊ดของเหมือนของคนนี้เลย
https://github.com/devmountaintechfest/hackathon-season2/pull/21/files
สรุปแล้วจะแข่งเดียวหรือแข่งเป็นทีมครับ?

@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

  1. มีการลง library เยอะมากเลย
  2. ถ้าจะเอาไว้ read data ทำไมถึงไม่เลือกใช้​ https://docs.python.org/3/library/xml.etree.elementtree.html แทนครับ?

ข้อ 1 ส่วนใหญ่เป็น lib ที่มีลงไว้ทำพวก vistualize ครับ ลงตัวนึงมันก็มี dependency มา
ข้อ 2 จากที่อธิบาย ตัว lib เอง ไม่ได้ เป็น utility convert json เหมือนตัวอื่นๆ ที่มีหลายๆ ตัว ใช้แค่ read xml เข้าใจว่าไม่เข้าข้อห้ามครับ ถ้าบอกว่าใช้ standard แต่แรก ก็ไม่ใช้แล้วครับ อันนกฏมันเทาๆ

คราวหน้าเขียนรบกวน ระบุกฎให้มันชัดๆ เลยก็ดีนะครับ ว่าใช้ ได้แต่ standard lib อย่างอื่นห้าม

ปล.
ของ element tree ก็ใช้งานคล้ายๆ กันครับในการ read
image

ปล อีกอัน
ทีมเดียวกันครับ

@annibuliful
Copy link
Collaborator

  1. มีการลง library เยอะมากเลย
  2. ถ้าจะเอาไว้ read data ทำไมถึงไม่เลือกใช้​ https://docs.python.org/3/library/xml.etree.elementtree.html แทนครับ?

คราวหน้าเขียนรบกวน ระบุกฎให้มันชัดๆ ด้วนนะครับ ว่าใช้ ได้แต่ standard lib ที่เขียนคือห้าม convert ไม่ได้ห้ามใช้อย่างอื่น เลยเข้าใจว่านอกจาก convert ใช้ได้ ส่วนที่ทักว่าเยอะเป็น lib ที่มีลงไว้ทำพวก vistualize ครับ

ปล อีกอัน ทีมเดียวกันครับ

ถ้าทีมเดียวกันผมจะขอตรวขของคุณอย่างเดียวนะครับ

@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

  1. มีการลง library เยอะมากเลย
  2. ถ้าจะเอาไว้ read data ทำไมถึงไม่เลือกใช้​ https://docs.python.org/3/library/xml.etree.elementtree.html แทนครับ?

คราวหน้าเขียนรบกวน ระบุกฎให้มันชัดๆ ด้วนนะครับ ว่าใช้ ได้แต่ standard lib ที่เขียนคือห้าม convert ไม่ได้ห้ามใช้อย่างอื่น เลยเข้าใจว่านอกจาก convert ใช้ได้ ส่วนที่ทักว่าเยอะเป็น lib ที่มีลงไว้ทำพวก vistualize ครับ
ปล อีกอัน ทีมเดียวกันครับ

ถ้าทีมเดียวกันผมจะขอตรวขของคุณอย่างเดียวนะครับ
รับทราบครับ ขอบคุณครับ

@annibuliful
Copy link
Collaborator

งั้นผมขอปิด PR ของเพื่อนคุณและตรวจแค่ PR นะครับ

@annibuliful annibuliful mentioned this pull request Oct 9, 2022
Closed
@annibuliful
Copy link
Collaborator

@khun-api
PR เป็นเพื่อนคุณด้วยรึเปล่าครับ?
#19

  1. มีการสร้าง data model ที่เหมือนกัน

Screen Shot 2565-10-09 at 22 16 19

  1. โค๊ดมีการเขียนเกือบจะเหมือนกันเลย

Screen Shot 2565-10-09 at 22 12 02

@annibuliful annibuliful mentioned this pull request Oct 9, 2022
@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

@khun-api PR เป็นเพื่อนคุณด้วยรึเปล่าครับ? #19

  1. มีการสร้าง data model ที่เหมือนกัน

Screen Shot 2565-10-09 at 22 16 19
  1. โค๊ดมีการเขียนเกือบจะเหมือนกันเลย
Screen Shot 2565-10-09 at 22 12 02

ใช้ PR 18 ตัวเดียวครับ ขอโทษด้วยครับ
ทีมมี 3 คน

khun-api
sintop
mofostupid

@annibuliful
Copy link
Collaborator

ผมเข้าใจตรงจุดนี้ครับว่าเขียนเหมือนเหมือนๆกัน แต่ lxml มี xpath() ที่ส่งแค่ selector แล้วสามารถได้ข้อมูลได้ง่ายกว่าการใช้ xml ที่เป็น standard library

lxml xml
310424253_444397307571206_263325384468336026_n 310562277_1713465422358868_1485199201494027565_n

ผมไม่เก่ง python นะ ผมถามจากเพื่อนเอา มันก็เหมือน DOM manipulation ที่มีพวก getElementById, getElementByClassname.
ถ้าเราไม่ใช้อะไรพวกนั้นมันก็คือ string ตรงๆแล้วมาสร้างเป็น object ก็ทำได้ยากระดับนึง มันก็เหมือน lxml กับ xml แล้วครับ

Screen Shot 2565-10-09 at 23 39 21

นี้น่าจะเป็น definition ที่เข้าใจง่ายที่สุดเท่าที่ผมจะหาได้ + ลองเล่น

เรื่องที่เราคุยกันต่างๆผมจะไม่เก็บตัวหักคะแนนเพราะมันไม่ได้อยู่ใน criteria ในการให้คะแนน
แต่เรื่อง lxml กับ xml ผมจะคุยกับทีมงานที่เขียน python ว่าปล่อยผ่านได้หรือใหม่

Screen Shot 2565-10-09 at 23 43 20

และพวกผมก็ยังสงสัยว่าทำไมต้องส่ง PR แยกกันในเมื่ออยู่ทีมเดียวกัน??? 🤔🤔🤔🤔🤔🤔🤔🤔

@khun-api
Copy link
Author

khun-api commented Oct 9, 2022

ผมเข้าใจตรงจุดนี้ครับว่าเขียนเหมือนเหมือนๆกัน แต่ lxml มี xpath() ที่ส่งแค่ selector แล้วสามารถได้ข้อมูลได้ง่ายกว่าการใช้ xml ที่เป็น standard library

lxml xml
310424253_444397307571206_263325384468336026_n 310562277_1713465422358868_1485199201494027565_n
ผมไม่เก่ง python นะ ผมถามจากเพื่อนเอา มันก็เหมือน DOM manipulation ที่มีพวก getElementById, getElementByClassname. ถ้าเราไม่ใช้อะไรพวกนั้นมันก็คือ string ตรงๆแล้วมาสร้างเป็น object ก็ทำได้ยากระดับนึง มันก็เหมือน lxml กับ xml แล้วครับ

Screen Shot 2565-10-09 at 23 39 21

นี้น่าจะเป็น definition ที่เข้าใจง่ายที่สุดเท่าที่ผมจะหาได้ + ลองเล่น

เรื่องที่เราคุยกันต่างๆผมจะไม่เก็บตัวหักคะแนนเพราะมันไม่ได้อยู่ใน criteria ในการให้คะแนน แต่เรื่อง lxml กับ xml ผมจะคุยกับทีมงานที่เขียน python ว่าปล่อยผ่านได้หรือใหม่

Screen Shot 2565-10-09 at 23 43 20

และพวกผมก็ยังสงสัยว่าทำไมต้องส่ง PR แยกกันในเมื่ออยู่ทีมเดียวกัน??? 🤔🤔🤔🤔🤔🤔🤔🤔

ขอบคุณครับ​ ผมส่งรูป​ ที่ปรับเป็น​ element tree ซึ่งต่างกันไม่เยอะครับ​ XPath และได้​ object ได้เหมือนกัน​

lxml obj.xpath
Element tree obj.find

xml ที่ส่งมาด้านบนคือการ travel node เอง​ ซึ่งเป็นวิธีดั้งเดิม​ แต่มีอีกวิธีที่นิยม​ คือ​ XPath ครับ​

แต่อาจจะเขียน​ต่างกันบางส่วนครับ​ ซึ่งปรับแล้วทำงานต่อจาก response ได้เลย​ครับ ส่วนเรื่อง​ pr สื่อสารกันในทีมไม่ดีเองครับ​ ขอโทษที่ทำให้สับสนครับ

@annibuliful
Copy link
Collaborator

ได้ครับ เดี๋ยวจะเอาตรงจุดนี้ไปปรึษาากับทีมครับ ขอบคุณครับ

Comment on lines +53 to +54
for i in range(500):
nationsResult.append([])
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have no idea why you have to append empty array 500 items

Comment on lines +60 to +67
if(emp[6] not in nationIndex.keys()):
nationIndex[emp[6]]=index
currentIdex=nationIndex[emp[6]]
nationsResult[currentIdex].append(emp)
index=index+1
else:
currentIdex=nationIndex[emp[6]]
nationsResult[currentIdex].append(emp)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems hard to read what is this logic using for

Comment on lines +188 to +214
if (exeType=='M'):
datasource = sys.argv[2]
dbName = sys.argv[3]
csvPath = sys.argv[4]
reportName = sys.argv[5]
config={
"datasource":datasource,
"dbName":dbName,
"csvPath":csvPath,
"clubDataReport":reportName
}

startTime = time.perf_counter()

exe=Executor()
exe.setup(config)
exe.extract()
exe.transform()
exe.generateCSVByNationality()
# exe.load()
exe.loadFromCSV()
exe.generateSummary()


endTime = time.perf_counter()
totalTime = endTime - startTime
print(f'##Total used time {totalTime:.4f} seconds##')
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of long code block condition I think you can wrap up with function it make more readable

Comment on lines +215 to +243
elif (exeType=='Q'):
dbName = sys.argv[2]
searchType = sys.argv[3]
valueSearch = sys.argv[4]
if(searchType!='' or valueSearch!=''):
config={
"searchType":searchType,
"dbName":dbName,
"valueSearch":valueSearch
}
startTime = time.perf_counter()

exe=Executor()
exe.setup(config)
if searchType=='region':
exe.queryByRegion(valueSearch)
elif searchType=='dept':
exe.queryByDepartment(valueSearch)
elif searchType=='nationality':
exe.queryByNationality(valueSearch)
else:
print("Invalid Parameter!")
printInfoQ()
else:
print("Please enter Parameter!")
printInfoQ()
endTime = time.perf_counter()
totalTime = endTime - startTime
print(f'##Total used time {totalTime:.4f} seconds##')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

@@ -0,0 +1,83 @@
import sqlite3
Copy link
Collaborator

@annibuliful annibuliful Oct 10, 2022

Choose a reason for hiding this comment

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

I really love to this file it's very easy to read and understand

import time
from datetime import datetime
from datetime import date
from dateutil import relativedelta
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants