-
Notifications
You must be signed in to change notification settings - Fork 172
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
Remove dependency to setoptconf #681
base: master
Are you sure you want to change the base?
Conversation
3651581
to
cbb558e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing deps is always nice.
Not sure if adding pydantic to the deps tree is advisable as we already have a lot of dependencies to consider (and there's also the user depencency in prod) but ultimately you choose :)
69f0834
to
cf82c0a
Compare
Pydentic is a very popular library to add types on configuration, it's true that it can interfere with user dependency, I will use a wider version range to avoid that, If you are against that, I can also use TypedDict, but this will not check the input this is only for type checking. |
ddcc442
to
d495b9d
Compare
Currently, this is not working, but what do you think about using pydentic for data validation, if we use it, I plane to also use it to validate the profiles. @carlio ? |
614c09d
to
e56465c
Compare
3dc8e4b
to
b8a5bf1
Compare
@sbrunner I just discovered tyro and minterface which might help replacing setoptconf. Just ideas for you. Also clipstick and others linked from the tyro README |
b8a5bf1
to
5fea080
Compare
With clipstick I didn't succeed to create command like |
5fea080
to
fef672f
Compare
With tyro I will get help like that
This is nice, but the boolean and list are not threaded as expected :-/ |
Shouldn't I continue to improve the couple Pydentic + argparse? |
df8356e
to
19a7460
Compare
aa63d6e
to
95538e0
Compare
Use whichever works best, I don't mind :) I just saw those and thought they might help, but I guess not. |
Tyro uses pydantic anyway though you might be able to patch the help formatter if that's the only concern |
95538e0
to
4481479
Compare
The issue with tyro is not the help it that he expected that we use, e.g.: |
4481479
to
2977c81
Compare
For me, the current solution is the best I tested :-) |
Do You think that I can merge this pull request, from now I don't see any better solution |
Description
Prototype to remove setuptconf, do you like it, should I go ahead with that?
Related Issue
fix #442