Bug: art.rewards.ruler function and module collision
Opened this issue · 3 comments
Art version: 0.4.4
Python: 3.12.11the function art.rewards.ruler.ruler() and the module itself art.rewards.ruler have the same name.
Since in the art.rewards.__init__ you import the function art.rewards.ruler.ruler() as "ruler", a collision occurs between the module name and the function.
To demonstrate:
(1)
import art.rewards.ruler as art_ruler
print(art_ruler.DEFAULT_RUBRIC)(2)
from art.rewards import ruler as art_ruler
print(art_ruler.DEFAULT_RUBRIC)(3)
import art.rewards.ruler
print(art.rewards.ruler.DEFAULT_RUBRIC)All Resulting in the error:
AttributeError: 'function' object has no attribute 'DEFAULT_RUBRIC'Basically, I dont even think it's possible to somehow import DEFAULT_RUBRIC, or anything which is defined inside the
art.rewards.ruler module while not being imported in art.rewards.__init__.
fixing it will be a breaking change either way I believe, so I would suggest renaming the function art.rewards.ruler.ruler() to something else, as most likely those who use RULER probably use it from the ruler_score_group() function.
Hi @giladfrid009,
Do you find it useful to be able to import DEFAULT_RUBRIC? It was originally meant for internal use, which is why we didn’t expose it.
I’m not sure we’ll be adding enough new utilities to that file to justify a refactor.
Im trying to create a config class which holds RULER config (the idea is to serialize it). ideally the default value of the rubric in that config would've been DEFAULT_RUBRIC. I can solve this issue in other ways, but they are not ideal.
An easy alternative which solves at least my issue and is not a breaking change - is to allow a None value for the rubric parameter in the functions ruler() and ruler_score_group() - in case of None a default rubric would be used, e.g:
def ruler(...)
if rubric is None:
rubric = DEFAULT_RUBRICBut i believe the issue is wider, as any method / class declared in art.rewards.ruler and not explicitly mentioned in art.rewards.__init__ can't be imported, which is not ideal.
Hey @Kovbo, any update?
If no changes are planned w.r.t this issue feel free to close this.