[1332][infra] Remove getattr() and prevent future use#1739
[1332][infra] Remove getattr() and prevent future use#1739simone99n wants to merge 1 commit intoecmwf:developfrom
Conversation
|
@simone99n : related issue but maybe you have some insights since you are working on this: we currently have a gap between linting locally (using scripts/actions.sh lint) and what happens on github. Specifically, if class members are not initialized in the ctor then github linting throws an error but local linting does not. Any idea where this comes from and how to fix it? |
Did you try also to run The Github workflow for the linting is the following:
|
And what is run with |
I may be missing something, but I don’t understand why we have two different linting approaches. |
I think |
It makes sense to me 👍🏻 |
Description
In this DRAFT PR, we remove the use of
getattr()from several places:tokenizer_masking.pyThe use of
getattr()was unnecessary, sinceself.rng = Noneis explicitly defined in__init__().loss_calculator.pyandloss_module_physical.pyWe now rely on
__dict__instead ofgetattr(). However, this approach is still not safe.loss_fnsandLossModules.test_cli.pybetter_abc.pyAdditionally, in
pyproject.toml, I attempted to enable automatic detection ofgetattr()usage, but it is currently not working as expected.Issue Number
Checklist before asking for review
./scripts/actions.sh lint./scripts/actions.sh unit-test./scripts/actions.sh integration-testlaunch-slurm.py --time 60