Skip to content

Post petsctools.Options improvements to OptionsManager - #57

Merged
connorjward merged 9 commits into
mainfrom
connorjward/options-manager-fix
Oct 9, 2026
Merged

connorjward merged 9 commits into
mainfrom
connorjward/options-manager-fix

Conversation

@connorjward

Copy link
Copy Markdown
Collaborator
  • Use petsctools.Options as OptionsManager.options_object. This means that we can get the right types with petsctools.Options.__getitem__ later on.
  • Remove AppContextManager. Since petsctools.Options DTRT when we pass Python options we can rely on it to correctly push and pop things from the global state.

This means that we get the right option types when we read from
petsctools.Options later on.
Comment thread petsctools/options.py Outdated

@JHopeCollins JHopeCollins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Every time I think we're converging on the right thing, you manage to find a way to simplify it!

Looks like now the only place we need to worry about whether options are native or python types is in petsctools.Options?

Comment thread petsctools/options.py
Comment thread petsctools/options.py Outdated
Comment thread petsctools/options.py
Comment thread petsctools/options.py
Comment thread tests/test_options.py
@connorjward

Copy link
Copy Markdown
Collaborator Author

Looks like now the only place we need to worry about whether options are native or python types is in petsctools.Options?

Yep!

connorjward and others added 2 commits October 9, 2026 15:14
Co-authored-by: Josh Hope-Collins <jhc.jss@gmail.com>
@connorjward

Copy link
Copy Markdown
Collaborator Author

Turns out that

>>> print(PETSc.Options("").prefix)
None

so the self.prefix or "" is needed

@JHopeCollins

Copy link
Copy Markdown
Member

Turns out that

>>> print(PETSc.Options("").prefix)
None

so the self.prefix or "" is needed

That is rather vexing

@connorjward

Copy link
Copy Markdown
Collaborator Author

Turns out that

>>> print(PETSc.Options("").prefix)
None

so the self.prefix or "" is needed

That is rather vexing

https://gitlab.com/petsc/petsc/-/merge_requests/9697

@JHopeCollins JHopeCollins left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm very happy with this, thanks!

@connorjward
connorjward merged commit 52056c5 into main Oct 9, 2026
2 checks passed
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