<fix> 함수 내 module import 관련 issue

olenmg commented 3 years ago

def get_retriever(args):
    if args.model.retriever_name == "tfidf":
        from konlpy.tag import Mecab

        mecab = Mecab()
        retriever = SparseRetrieval(args, tokenize_fn=mecab.morphs)
    return retriever

위와 같이 함수 내 분기에 따라 module을 import 하는 방식에 대한 의견을 여쭙고 싶습니다!

모듈 상단에서만 import할 시

코드가 깔끔해짐
속도가 조금 더 느림

모듈 내 분기에 따라 import할 시

코드가 좀 난잡해보일 수 있음
분기를 거치지 않을 경우 해당 모듈 import하지 않으므로 속도에 이점이 있음

ggm1207 commented 3 years ago

제가 코드를 작성한 입장으로써 쓸데 없는 혼란을 야기한 것 같아서 죄송하지만 (의외로) 저는 모듈 상단에 import 하는 것이 옳다고 봅니다! 사실 그렇게 속도면에서 차이가 있지도 않습니다. ㅠㅜ

혼란을 잠재우기 위해 저렇게 작성한 이유를 몇 자 적어보자면..

리트리버가 리팩토링 전이라 tokenize_fn 에 인자를 넘겨줘야 됐기 때문에 어쩔 수 없이 선언을 해야 했고
Mecab이 prepare 상단에 import 되기 보다는 SparseRetrieval를 위한 모듈이라 그것을 부각시키기 위함도 있습니다!
즉, 나중에 SparseRetrieval 리팩토링 하면서 Mecab이 prepare.py에 선언되기 보다는 그 쪽에서 직접 선언되면 좋다는 생각을 했습니다!

요약하자면

속도에 그렇게 큰 영향을 미치지 않는다면 상단에 import
위 코드는 추후 리팩토링때 해결 될 거라고 생각하고 주먹구구식으로 짠 코드 ㅠㅜ

olenmg commented 3 years ago

앗 제 의견은 안남겼었는데 저도 Mecab 모듈 같은 경우는 실행에 오랜시간이 걸리진 않아서 저도 위에서 import하는게 좋을 것 같습니다!

ggm1207 commented 3 years ago

앗 제 의견은 안남겼었는데 저도 Mecab 모듈 같은 경우는 실행에 오랜시간이 걸리진 않아서 저도 위에서 import하는게 좋을 것 같습니다!

VumBleBot / odqa_baseline_code

<fix> 함수 내 module import 관련 issue #9