Skip to content

[WIP] Converter System clean-up#355

Draft
N-Coder wants to merge 2 commits intomainfrom
impr-conv-ctx
Draft

[WIP] Converter System clean-up#355
N-Coder wants to merge 2 commits intomainfrom
impr-conv-ctx

Conversation

@N-Coder
Copy link
Copy Markdown
Member

@N-Coder N-Coder commented Jul 11, 2022

This PR tracks the new Converter System, which is currently the biggest blocker for v0.8 (see #245).
The first commit takes the first big step towards a more ergonomic (and esp. extensible) converter hierarchy, sketching how the initialization around the contextmanager-local, non-threadsafe ConverterContext and its Converter(Factory) mapping works. A few things that are now nicer than before

  • Converters can store their data locally as fields instead of using the unwieldy context dict together with obfuscated keys
  • the extra context param has been dropped everywhere, the few places that explicitly need it get it from a thread local variable
  • all context data together with the current converters and their configuration are now collected within ConverterContext, which should make managing this easier (less weird keys in dicts flying around)
  • things are still thread-safe (or even more so) as different threads have different Converter(-Context) instances
  • this even allows for different configurations with different converters (e.g. compliance to different RFCs / versions) in the same process
  • customization should be a lot easier as fewer things are immutable
  • initialization is no longer based on module imports, so this should also be easier to handle and extend
  • I removed some complexity around ImmutableComponentMeta and __attrs_post_init__ for immutable classes
  • the whole systems now makes a lot more sense in my head and should be easier to explain (while still being somewhat complicated, but I guess some of that is inherent)
  • ValueConverters are now singletons and thus easier to handle
  • ...probably more things which I forgot

The specific Converters still need to be patched to use the new context to store data. So there's still further clean-up to do before this even compiles, and then this needs to be tested, and I also want to include some basic documentation on what I did and how extensions could work. Maybe I even have time to polish some further things like removing the intermediate attrs classes now that we can customize __init__ and the RuntimeAttrValidation that is now built-in to attrs. I'll update this PR once I got to all (or at least some) of that.

This leaves error-handling / variable strictness and recurring events on the ToDo list, but I guess we can just as well defer these features for 0.9 as long as we get these pretty deep-rooted changes here through before anybody strongly relies on how the converters work.

PS: sorry for opening this so shortly before the "blackening", I really wanted to get these changes finally visible. Still, I guess this shouldn't block the mass-reformat, as I can probably "rebase" my changes applying black to every/the single commit here.

This is the first step towards a more ergonomic (and esp. extensible) converter hierarchy, sketching how the initialization around the contextmanager-local, non-threadsafe ConverterContext and its Converter(Factory) mapping works. The specific Converters still need to be patched to use the new context to store data.
@N-Coder N-Coder added this to the Version 0.8 milestone Jul 13, 2022
@C4ptainCrunch C4ptainCrunch marked this pull request as draft July 15, 2022 10:58
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.

1 participant