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

Replace setoptconf #442

Open
carlio opened this issue Sep 8, 2021 · 11 comments · May be fixed by #681
Open

Replace setoptconf #442

carlio opened this issue Sep 8, 2021 · 11 comments · May be fixed by #681

Comments

@carlio
Copy link
Member

carlio commented Sep 8, 2021

setoptconf is used for loading configuration values but is an old, stale library; #438 shows that it needs to be updated, or an alternative found.\

(for now a temporary solution is in place)

This issue is here as a placeholder for work done to properly replace it with another library (or alternatively take on maintaining setoptconf completely but... I really don't want to do that).

(ping @jayclassless )

@jayclassless
Copy link
Contributor

Oh wow -- a blast from the past. :)

If you send over a PR with the fixes you made, I'd be happy to merge them and publish an update to PyPI to hold you over until you're able to completely replace it.

@graingert
Copy link

@jayclassless thanks I made a PR jayclassless/setoptconf#3

@jayclassless
Copy link
Contributor

@jayclassless thanks I made a PR jayclassless/setoptconf#3

Merged and published as 0.3.0.

@graingert
Copy link

@jayclassless nice, can you also upload a wheel so nobody will be impacted by future setuptools changes?

@jayclassless
Copy link
Contributor

Yup, good call. Uploaded.

@sbrunner
Copy link
Member

sbrunner commented Oct 5, 2024

Shouldn't we use pydentic?
https://realpython.com/python-pydantic/

@carlio
Copy link
Member Author

carlio commented Oct 6, 2024

Correct me if I'm wrong, but pydantic will not do the command line argument parsing as well? It's not simply about reading the .prospectorrc config files, but having a unified way to also handle prospector --some-arg where some-arg could also be in a config file. I started looking into Click for this and ran into the same issue but the other way round, not sure if I investigated either option enough though.

@sbrunner
Copy link
Member

sbrunner commented Oct 7, 2024

Effectively, I completely missed this part... but after a small search I see that there is a project that does that:
https://pypi.org/project/argparse-pydantic/
I'd like to start a test with that :-)

@carlio
Copy link
Member Author

carlio commented Oct 7, 2024

That looks pretty neat, it's worth trying.

One concern though - just looking at the README, it gives the example of arg_int which is turned into the CLI option --arg_int, while prospector has flags like --absolute-paths. As long as there's a way to keep the hyphens instead of underscores for CLI usage, then great, because people won't have to change their CI scripts. However you obviously can't have a Python class member called absolute-paths

@sbrunner
Copy link
Member

sbrunner commented Oct 7, 2024

+1 This is an important point, this should be solved :-)

@carlio
Copy link
Member Author

carlio commented Oct 7, 2024

@sbrunner sbrunner linked a pull request Oct 17, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants