clib.session: Refactor to simplify the checking of currently open session#3523
clib.session: Refactor to simplify the checking of currently open session#3523
Conversation
seisman
added
maintenance
| the context manager). | ||
| """ | ||
| if not hasattr(self, "_session_pointer") or self._session_pointer is None: | ||
| if getattr(self, "_session_pointer", None) is None: |
There was a problem hiding this comment.
if not hasattr(self, "_session_pointer") or self._session_pointer is None: is equivalent to if getattr(self, "_session_pointer", None) is None:.
| @@ -338,19 +338,12 @@ def create(self, name: str): | |||
| name | |||
| A name for this session. Doesn't really affect the outcome. | |||
| """ | |||
There was a problem hiding this comment.
If self.session_pointer is defined and is not None, then we know we have an open session. The try..except statement was added in 2018 (#210) and is not necessary.
There was a problem hiding this comment.
As mentioned in #3448 (comment), a with contextlib.supress block should ideally be used when there is only one line below it. I feel like we should keep the previous try-except block.
There was a problem hiding this comment.
It's impossible that the msg assignment and the raise GMTCLibError lines can raise a GMTCLibNoSessionError exception, so we should be safe.
Edit: Read astral-sh/ruff#1947 again, actually it's unclear if the raise GMTCLibError statement will be executed or not. I think we have to go back to try..except.
There was a problem hiding this comment.
Reverted back to try..except in da9c5e1 and improve the comments to make it easier to understand.
seisman
force-pushed
the
refactor/session-pointer
branch
from
b960654 to
42ae467
Compare
seisman
force-pushed
the
refactor/session-pointer
branch
from
42ae467 to
3d0c91b
Compare
seisman
added
final review call
Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
seisman
removed
the
final review call
3 participants
Just some refactoring to simplify the codes. See the changes for more details.