Skip to content

Update Config to support ConfigParser keyword arguments#659

Merged
jph00 merged 2 commits intomainfrom
config_parser
Dec 26, 2024
Merged

Update Config to support ConfigParser keyword arguments#659
jph00 merged 2 commits intomainfrom
config_parser

Conversation

@ncoop57
Copy link
Copy Markdown
Contributor

@ncoop57 ncoop57 commented Dec 22, 2024

This PR adds a cfg_kwargs argument to the Config object in the foundations module. This allows a user to pass keyword arguments directly to the ConfigParser object, allowing for things like overriding the behavior on how inline comments are handled. An example showcasing this functionality is also added to the docs.

@ncoop57 ncoop57 requested a review from jph00 December 22, 2024 19:36
@ncoop57
Copy link
Copy Markdown
Contributor Author

ncoop57 commented Dec 22, 2024

The integration test failure looks to be unrelated

@jph00
Copy link
Copy Markdown
Contributor

jph00 commented Dec 22, 2024

Yeah the integration test issue is due to a new bug in scipy, which has been fixed upstream but not released yet.

Copy link
Copy Markdown
Contributor

@jph00 jph00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Just a minor issue below.

Comment thread nbs/02_foundation.ipynb
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you install ipywidgets (IIRC) and re-run the cell with /Users/nathan/.venv/lib/python3.12/site-packages/tqdm/auto.py:21: TqdmWarning: IProgress not found. Please update jupyter and ipywidgets. See https://ipywidgets.readthedocs.io/en/stable/user_install.html so that warning doesn't turn up in the docs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this is fixed now, good catch!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jph00 anything else needed to get this merged in?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry got a bit behind over the xmas period! :O

@jph00 jph00 merged commit e4335a1 into main Dec 26, 2024
@jph00 jph00 added the enhancement New feature or request label Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants