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

Make config file optional #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benmuth
Copy link

@benmuth benmuth commented Jul 4, 2024

This fixes #29.

Problems this solves:

  • users have control over if/how they want to use the config file
  • config file path is customizable
  • certain config file options weren't being used at all (for example, the verbose setting was always overwritten by the CLI)

Hopefully this doesn't break any use-cases/workflows. Let me know if there's anything you're skeptical about.

@MicheleCotrufo
Copy link
Owner

MicheleCotrufo commented Aug 9, 2024

Hey, sorry that it took me so long to look at this. I was looking at your suggested code, and I noticed that you moved the call of the function config.ReadParamsINIfile(config_path) to the main() function inside the main.py file. This should work well when pdf2doi is invoked via command line, but I'm not sure if it would work well when pdf2doi is used as a library by other scripts (for example, from pdf2bib or pdf-renamer. Can you run some test?

@benmuth
Copy link
Author

benmuth commented Aug 19, 2024

When pdf2doi is used as a library, I think the calling code can just call pdf2doi.config.ReadParamsINIfile(config_path) during initialization (for instance, just before where pdf2bib and pdf-renamer are calling pdf2doi.config.set right now).

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.

Make config file optional
2 participants