search

shon-otmazgin / fastcoref

MIT License
142 stars 25 forks source link

prevent auto-installing wandb #27

Closed aryehgigi closed 1 year ago

aryehgigi commented 1 year ago

Fixes #26

aryehgigi commented 1 year ago

@shon-otmazgin @ariecattan wdyt? this is ready for review

shon-otmazgin commented 1 year ago

@aryehgigi very nice and clean solution. before merge, did you test it and it worked?

aryehgigi commented 1 year ago

actually i didnt check it. and now that i do i see that it does not install wandb unless specified - as expected. but then from fastcoref import FCoref throws an error cause __init__.py imports both modeling stuff and training stuff. we can remove it from the init, but then users would need to import like this: from fastcoref.modeling import FCoref which is fine, but will break backward compatability.

so dont merge this yet as i dont have a solution to not break backward compatibility but in the same time not import training. if you find something lmk?

aryehgigi commented 1 year ago

ok @shon-otmazgin i found a solution that doesnt break backward compatability and doesnt import wandb if the trainer is not used. the only disadvantage is that it is a bit ugly code-wise. but personally i think its fine.. wdyt? (i obviously tested it now)