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

pyproto: deprecate protocol traits #2173

Merged
merged 4 commits into from
Feb 24, 2022

Conversation

davidhewitt
Copy link
Member

The final step before I think we're ready to release 0.16!

@davidhewitt
Copy link
Member Author

@birkenfeld to answer your question in #2159 (comment) - I started this branch last night. If you want to work on it please do take it over and push further commits! I'll get working on the 0.16 release branch instead.

I guess the remaining work here is to go through documentation, generally making #[pyproto] less obvious, and wherever it does exist point users to #[pymethods] instead?

@birkenfeld
Copy link
Member

Ok, I'll get on it. BTW, it is sad to see all the #![allow(deprecated)]; I'd have thought that deprecations in the same crate would be allowed, as this will be easy to overlook needing removal later.

@davidhewitt davidhewitt mentioned this pull request Feb 17, 2022
@davidhewitt
Copy link
Member Author

Agreed, though I think most of these files can simply be deleted when we finally remove #[pyproto]

@birkenfeld
Copy link
Member

So, questions that came up while working on this:

  1. Is __getattr__ actually equivalent to Python __getattr__ or rather __getattribute__?
  2. In pyproto's __getitem__ etc., negative indices were handled specially. I assume I was correct that this is not done for pymethods?
  3. There doesn't seem to be support for __inplace_concat__ and __inplace_repeat__ sequence methods?

@davidhewitt
Copy link
Member Author

  1. Yes, the two methods share the one slot, however we have this code which makes the slot behave like __getattr__. I think in principle we could support both methods like CPython.

  2. Yes, the summary of the issue is that if the Py_sq_len slot is implemented then CPython will add that length to a negative index before accessing the sequence slots. The mapping slots are always tried first, so in practice this means that only in weird edge cases (from C) would CPython do this adjustment to negative indices. I felt this was hard for users to understand and impossible to correct for, so I chose for our __len__ to not implement the Py_sq_len slot and thus minimize potential confusion.

  3. Ooh, good spot. I think I missed because these magic methods don't exist in pure Python; they're just lumped in with __iadd__. We could copy the existing design straight from #[pyproto], seems clear enough?

@davidhewitt
Copy link
Member Author

Pushed a commit to add __inplace_concat__ and __inplace_repeat__ support.

@davidhewitt
Copy link
Member Author

I'll merge this tonight if nobody has any concerns!

@birkenfeld
Copy link
Member

birkenfeld commented Feb 23, 2022

Nice, __getattribute__ can then be another PR.

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