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

Python 3.11: it's __notes__, not __note__ #301

Closed
henryiii opened this issue Sep 12, 2022 · 2 comments · Fixed by #303
Closed

Python 3.11: it's __notes__, not __note__ #301

henryiii opened this issue Sep 12, 2022 · 2 comments · Fixed by #303

Comments

@henryiii
Copy link
Contributor

henryiii commented Sep 12, 2022

cattrs is using __note__, but it's actually __notes__, and a tuple of strings, and should be (python version conditionally) added via .add_note(string), perhaps? (depending on the backport, I guess). exceptiongroup was updated in 1.0.0rc4, see https://github.com/agronholm/exceptiongroup/blob/main/CHANGES.rst.

Also, the message seems pretty poor on pytest + Python 3.10, is pytest and exceptiongroup clashing?

      except Exception as e:
        e.__note__ = 'Structuring class Index @ attribute reply'
        errors.append(e)
>     if errors: raise __c_cve('While structuring Index', errors, __cl)
E     cattrs.errors.ClassValidationError: While structuring Index (1 sub-exception)

<cattrs generated structure scikit_build_core.file_api.model.index.Index>:19: ClassValidationError

(There's no more helpful err message below it, just that)

I tried running this outside of pytest, and it's much better, I can see the errors. But not inside pytest. I'm guessing it's the monkey patching warnings in https://pypi.org/project/exceptiongroup/. Maybe something to bring up with pytest? Or exceptiongroup? I think that might be agronholm/exceptiongroup#23.

This is really cool when it works, though! (though the notes are missing, due to this issue, unless I force exceptiongroup==1.0.0rc3)

@Tinche
Copy link
Member

Tinche commented Sep 12, 2022

You're right of course, but I don't think we need to support both versions. We should depend on the first exceptiongroup version to have __notes__ and just use that (probably using __notes__ directly, to avoid the overhead of a method call ;). I would also be ok depending on the latest backport version, I don't think it's a huge burden on users to upgrade to it. Feel like making that PR?

As for pytest, I've noticed the same thing. It's probably on their end, not sure yet.

@henryiii
Copy link
Contributor Author

Sure, I can.

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 a pull request may close this issue.

2 participants