Moved compile at the end and modified its content. #2113
Conversation
There was a problem hiding this comment.
Thank you, the settings print looks good.
However, it makes it impossible to see the default values when trying to print the compilations options and core options directly.
We can add extra input to __repr__ if thye have default value, so we can have settings call compile's __repr__ with a flag to shorten it while leaving the default call printing everything.
qutip/core/options.py
Outdated
| @@ -34,7 +34,8 @@ def __setitem__(self, key, value): | |||
| def __repr__(self): | |||
There was a problem hiding this comment.
| def __repr__(self): | |
| def __repr__(self, full=True): |
There was a problem hiding this comment.
@Ericgig Thanks for your advice! I added 'full option'. When full=True, print(settings.compile.__repr__(full=True)) outputs everything. When full=False, it only outputs modified settings.
There was a problem hiding this comment.
Yes so now, print(settings.compile) will print the full output as expected. We still want to have print(settings) print the shorten version.
Also if you can, when no option is printed in the short version, it could print something like:
...
colorblind_safe: False
compile: <CompilationOptions()>
instead of
...
colorblind_safe: False
compile: <CompilationOptions(
)>
There was a problem hiding this comment.
Hi. I included your suggestions! You can try them;
>>> import qutip.settings as settings
>>> print(settings)
>>> print(settings.compile)
>>> settings.compile.__setitem__('accept_int',True)
>>> print(settings)
qutip/core/options.py
Outdated
| out += [")>"] | ||
| return "\n".join(out) | ||
| if cnt: |
There was a problem hiding this comment.
cnt is equal to len(out) - 2 and could be removed.
There was a problem hiding this comment.
Thanks! I removed cnt.
qutip/settings.py
Outdated
| if not attr.startswith('_'): | ||
| if attr != "core" and attr != "compile": | ||
| lines.append(f" {attr}: {self.__getattribute__(attr)}") | ||
| if "compile" in self.__dir__(): |
There was a problem hiding this comment.
The normal way to test if an attribute exist would be hasattr(self, "compile").
But compile is added in __init__ as None. So a better check would be if self.compile is not None. But it would only happen if for some strange reason, someone imported this file without importing qutip, so I would be fine with removing the check.
qutip/settings.py
Outdated
| lines.append(f" {attr}: {self.__getattribute__(attr)}") | ||
| if "compile" in self.__dir__(): | ||
| attr = "compile" | ||
| content = self.__getattribute__('compile').__repr__(full=False) |
There was a problem hiding this comment.
| content = self.__getattribute__('compile').__repr__(full=False) | |
| content = self.compile.__repr__(full=False) |
There was a problem hiding this comment.
Thank you. This is pretty nice!
qutip/settings.py
Outdated
| if not attr.startswith('_'): | ||
| if attr != "core" and attr != "compile": |
There was a problem hiding this comment.
Stacking if is not great.
We could simplify it by creating variables:
is_public_attr = not attr.startswith('_')
is_special_case = attr == "core" or attr == "compile"
if is_public_attr and not is_special_case:
or shorten the expression:
if not attr.startswith('_') and attr not in ["core", "compile"]:
There was a problem hiding this comment.
I chose the short expression. I removed all unnecessary variables from codes.
|
Not sure why, by there is a conflict stopping me from merging... |
|
@Ericgig I fixed the conflict now. |
|
@Ericgig I found that |
|
@Ericgig I fixed the conflict and a related problem. Could you see if everything is ok and merge? |
Checklist
Thank you for contributing to QuTiP! Please make sure you have finished the following tasks before opening the PR.
Description
I moved 'compile' at the end in qutip.settings and shortened it to only show the non-default values.
Related issues or PRs
#2100