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

Don't implicitly chain exceptions when accessing non-existent storage attributes #7734

Merged
merged 4 commits into from
Jul 13, 2023

Conversation

akihironitta
Copy link
Member

This PR changes the traceback when a user tries to access attributes on Data that doesn't exist:

import torch_geometric

data = torch_geometric.data.Data()
data.asdf

Before

Traceback (most recent call last):
  File "/workspaces/pytorch_geometric/torch_geometric/data/storage.py", line 80, in __getattr__
    return self[key]
  File "/workspaces/pytorch_geometric/torch_geometric/data/storage.py", line 105, in __getitem__
    return self._mapping[key]
KeyError: 'asdf'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/workspaces/pytorch_geometric/test.py", line 4, in <module>
    data.asdf
  File "/workspaces/pytorch_geometric/torch_geometric/data/data.py", line 475, in __getattr__
    return getattr(self._store, key)
  File "/workspaces/pytorch_geometric/torch_geometric/data/storage.py", line 82, in __getattr__
    raise AttributeError(
AttributeError: 'GlobalStorage' object has no attribute 'asdf'

After

Traceback (most recent call last):
  File "/workspaces/pytorch_geometric/test.py", line 4, in <module>
    data.asdf
  File "/workspaces/pytorch_geometric/torch_geometric/data/data.py", line 475, in __getattr__
    return getattr(self._store, key)
  File "/workspaces/pytorch_geometric/torch_geometric/data/storage.py", line 82, in __getattr__
    raise AttributeError(
AttributeError: 'GlobalStorage' object has no attribute 'asdf'

Alternative

By explicitly chaining exceptions, one of the lines in the error changes:

-       except KeyError:
+       except KeyError as e:
            raise AttributeError(
-               f"'{self.__class__.__name__}' object has no attribute '{key}'")
+               f"'{self.__class__.__name__}' object has no attribute '{key}'") from e
- During handling of the above exception, another exception occurred:
+ The above exception was the direct cause of the following exception:

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #7734 (e0a6800) into master (9bc7017) will decrease coverage by 0.36%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7734      +/-   ##
==========================================
- Coverage   91.93%   91.58%   -0.36%     
==========================================
  Files         452      452              
  Lines       25537    25534       -3     
==========================================
- Hits        23478    23385      -93     
- Misses       2059     2149      +90     
Impacted Files Coverage Δ
torch_geometric/data/storage.py 83.11% <ø> (ø)

... and 20 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@mananshah99 mananshah99 left a comment

Choose a reason for hiding this comment

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

Generally prefer chaining exceptions, but since this is a very user-facing attribute, the solution here seems very reasonable. Thanks!

@rusty1s rusty1s merged commit cd72557 into master Jul 13, 2023
18 checks passed
@rusty1s rusty1s deleted the refactor/chain-exceptions branch July 13, 2023 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants