search

python-attrs / cattrs

Composable custom class converters for attrs, dataclasses and friends.
https://catt.rs
MIT License
781 stars 108 forks source link

cattr vs. cattrs need documentation #471

Closed jdennis closed 6 months ago

jdennis commented 7 months ago

We have a lot of (legacy) code that imports cattr. As far as I can tell there is no installable package called cattr, however it appears that installing cattrs installs a package called cattr. I'm assuming cattr evolved into cattrs and that cattr is deprecated and cattr is provided by cattrs for backward compatibility. Despite my best attempts at web searches I could not find any information regarding cattr vs. cattrs. The project documentation should have some discussion of this. And most importantly document if there is any incompatibility between cattr and cattrs, in other words if importing code changes their import from cattr to cattrs is that sufficient to become "modern" or are their other code changes needed when migrating from cattr to cattrs.

Tinche commented 7 months ago

Howdy.

Sure, but I don't know where to put this. I think the only piece of documentation we have on this is under https://catt.rs/en/stable/history.html#id10. I guess attrs has this page, but the switch there was much more impactful.

The short story is that when attrs migrated away from primary using the attr package in favor of the attrs package (so I guess when the next-gen interfaces became ready), cattrs followed that lead.

The mismatch between cattrs (the PyPI name) and cattr (the Python package you import) has been here from the start (just like in attrs). The cattr package is not deprecated though, just like attr is not deprecated. Whether you migrate or not is up to you, but as a rule of thumb new stuff only lands in the cattrs package. Other than that, there should be no incompatibilities (unless you count the fact that the fully qualified name of the Converter is now cattrs.converters.Converter instead of cattr.converters.Converter, and I don't really). Changing the import is enough, but not required and will never be required unless you actually need new stuff.

Maybe calling this change out more prominently in the changelog would be enough.

jdennis commented 7 months ago

Thanks @Tinche that's very helpful, I appreciate it. I had seen the attrs page on it's history but it wasn't clear to me how tightly coupled cattrs was to attrs. Just as an aside with regards to attr vs. attrs that also is rather confusing. As for where one might add this doc I noticed that the project documentation does not have a FAQ, perhaps that might be a good location.

FWIW I'm in the process of trying to clean up legacy code by fixing what our dependencies are and the import statements that go along with them. Perhaps we're way behind (ha ha) but I'll wager I'm not the only to go through this exercise and when it came to attr vs. attrs and cattr vs. cattrs it was challenging to understand exactly what I needed to modify to become "modern".

Tinche commented 6 months ago

Thanks for the feedback. I don't necessarily think this would be a frequently asked question since I think this is the first time I'm answering it ;)

I think a page for migrations might be useful though, aggregating all breaking changes between releases and mitigation steps. I will look into adding this next year.

In the mean time, I'll just add an admonition to the changelog to call more attention to the package switch, and some context.

Tinche commented 6 months ago

Going to close this now, let me know if you have any other questions.