search

emanjavacas / pie

A fully-fledge PyTorch package for Morphological Analysis, tailored to morphologically rich and historical languages.
MIT License
22 stars 10 forks source link

Some functions could be static instead of exterior to classes #30

Closed PonteIneptique closed 5 years ago

PonteIneptique commented 5 years ago

I am basing a tool on pie, inheriting from most classes. My data input is somewhat different, so of course I need to change some functions.

There is a lot of places though where I think I am copying/pasting and correcting just because a function from the same module was used instead of making it a class method.

Example: in scorer, "compute_scores" is a function of the module. From what I see, this is the only place where it's used. If it were a staticmethod of Scorer, I would not need to rewrite the get_score function

https://github.com/emanjavacas/pie/blob/47be46f9d788a8f11ef559b1b23a932ec074a572/pie/models/scorer.py#L88-L123

https://github.com/emanjavacas/pie/blob/47be46f9d788a8f11ef559b1b23a932ec074a572/pie/models/scorer.py#L31-L41

If you agree, I could PR as I go along ;)

emanjavacas commented 5 years ago

Fine by me. If you feel you can reuse this code as is, it's of course better to refactor as you suggest! Hopefully it is not too much work for you.

emanjavacas commented 5 years ago

I am closing this but feel free to submit the PR whenever.