composer icon indicating copy to clipboard operation
composer copied to clipboard

Fix pre-commit checks failing on fresh checkout of dev

Open dblalock opened this issue 3 years ago • 7 comments

Fixes CO-883 using a type: ignore as suggested by @hanlint.

dblalock avatar Aug 16 '22 04:08 dblalock

Hm... I'm not getting this on dev... any ideas why? image

mvpatel2000 avatar Aug 16 '22 17:08 mvpatel2000

@mvpatel2000 I think you can replicate the issue if you try: pip install --upgrade datasets (to >=version 2)

hanlint avatar Aug 16 '22 17:08 hanlint

image I'm getting several issues with upgrade... @dblalock are you also seeing these?

mvpatel2000 avatar Aug 16 '22 17:08 mvpatel2000

@dblalock could you actually just resolve all the lint errors with datasets 2 and then update meta.yaml / setup.py to upgrade to datasets 2?

mvpatel2000 avatar Aug 16 '22 17:08 mvpatel2000

CC: @moinnadeem do we need any tests for upgrading datasets?

mvpatel2000 avatar Aug 16 '22 17:08 mvpatel2000

Let's hold off on tests for updating datasets (ie. I can't think of anything that'll go wrong by updating that)

We'll need regression tests for updating tokenizers and transformers because of potential throughput and accuracy degradations respectively. If updating datasets requires updating transformers or tokenizers, then we should hold off and regression test. Otherwise, LGTM!

moinnadeem avatar Aug 16 '22 17:08 moinnadeem

@dblalock I put a bit on your plate, so if you don't have the bandwidth for this, lmk and I'm happy to help out. I'd rather just do the full upgrade now though given its something we need to get to anyways

mvpatel2000 avatar Aug 16 '22 17:08 mvpatel2000

@bandish-shah can you approve / review this? You appear to be the only remaining member of "composer-team-admins"

dblalock avatar Aug 18 '22 20:08 dblalock