Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP switch from quinine to yahp #133

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

WIP switch from quinine to yahp #133

wants to merge 11 commits into from

Conversation

dlwh
Copy link
Member

@dlwh dlwh commented Apr 19, 2022

Closes #130 . Our friends at Mosaic have created a nice little config library called yahp. It's not perfect, but it's like 90% of what I'd want. All the config is in dataclasses now, and the yaml barely had to change.

As a bonus I made it so that we can handle just about every argument from TrainingArguments in HF and it's auto-synced with updates to hugging face.

I still need to:

  • merge a PR into yahp to support lists of lists
  • fix docs for the small changes to configs
  • run more tests
  • maybe some more cleanup?!?

Fixes Issues

#130

Unit test coverage

Are there unit tests in place to make sure your code is functioning correctly?

^^ situation is no worse, and we have some semblance of static type checking sometimes

Known breaking changes/behaviors

Configs need to change a bit, but mostly just removing "nulls" that should just be unspecified rather than null, imho.

@dlwh dlwh requested review from siddk and J38 April 19, 2022 07:09
@dlwh dlwh marked this pull request as draft April 19, 2022 07:09
@J38
Copy link
Contributor

J38 commented Apr 19, 2022

Oh man we'll have to change the graphic on the front page ...

@J38
Copy link
Contributor

J38 commented Apr 19, 2022

Any particular reason you favor yahp over quinine ? I guess overall yahp has a team maintaining/developing it and we probably have some degree of freedom to directly alter it if we have great ideas ... quinine just has one person and we probably have more freedom to alter it but still at the discretion of Karan ... so I don't really care either way

@dlwh
Copy link
Member Author

dlwh commented Apr 19, 2022

In addition to the points you mention, I like types and dataclasses that my IDE will recognize. I also ran into a limitation in quinine/cerberus a while back (though I've forgotten by now exactly what it was. it had to do with lists)

Both Cerberus and Quinine also seems somewhat abandoned? If it were just quinine, I'd just take it over if need be, but with cerberus also being abandoned that seemed not good.

Base automatically changed from dev to main August 17, 2022 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants