optax
optax copied to clipboard
Fix NamedTuples Formatting
fixes duplicate formatting for classes inheriting from NamedTuples as seen in #669
@vroulet most of the classes / states in transform.py and combine.py don't have proper docstrings. would it still be worthwhile to remove :members: for those files (many of those classes won't have much rendered bc of no docstring)
Hello @amosyou (sorry got too many mails didn't catch this notification). It's better not to have a docstring than having a confusing one. So yes, you can remove :members: to all classes and we may fill all the docstrings later (one file at a time probably).
@vroulet i removed the :members: from all the states with the exception of SAMState in sam.py since SAMState uses the chex.dataclass decorator instead of inheriting from NamedTuple. for consistency should we change that to be a namedtuple?
That's great, it will really make the doc more readable! For SAM's State let me handle that internally.
Could you also merge with main to see latest changes? (for example I'm not sure why States such as ClipState are displayed as alias of tuple instead of alias of EmptyState)
@vroulet apologies on the delay! been quite a month for me but we're back. ready for another review :)
apologies for the delay! It looks like a lot of restructuring happened since my last changes. I can open a new pull request that does these changes on the new transform subpackage?
thanks @amosyou ! Yes, that's fine, do it however way its most convenient for you 😄