You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Everything that's not a test in test_type_promotion.py is a mess right now. There are tons of data dictionaries and helper functions. Quite a few of them are needed throughout the test suite, so they should be moved to a more central location. Furthermore, the data has a lot of redundancies, and even so, to use it, you often have to do tons of dict lookups to get at what you want. I think the biggest problem here is that I used the string values of the dtypes as the main key in the dictionaries, meaning you always have to translate those to actual dtype objects to use them. We should just use the actual dtype objects instead. The only concern here is that the dtype objects might not be hashable. But I don't think this is an actual problem in practice. If it somehow does become a problem for some library, we can worry about it then, and whether it should be worked around in the suite or if we should just require hashability in the spec (we already require equality). Finally, the parameterization logic for the tests that are in test_type_promotion is a bunch of hard to read list comprehensions. They may become easier to read once we refactor the data. If not, we should clean them up as well. So to summarize
Move the reusable data and helper functions to a central location (I'm thinking a new file, like dtype_helpers.py). The input and output type mappings should also be moved, as they are also useful outside of just these tests.
Change the data to be keyed on the dtype objects themselves. I don't actually care if we keep the "i4" type names anywhere. Those are used in the spec, but they aren't actually that clear. I would rather we just use the dtype name itself, like "int32", everywhere, even in the places where the string names are currently used (I think it's only in the type promotion parameterization keys).
Other refactors like renaming variables to better names are also welcome.
Clean up the parameterization logic in test_type_promotion.py
RE hashability, worst case there's probably a hack we can do in _array_module to wrap dtypes in hashable items (i.e. use dtype string names as hashes). This might be useful anyway so we can get consistent str(dtype) output for things like error messages and pytest parameters.
That would mean we'd have to unpack the dtype object everywhere it is used, though. Let's not worry about this unless/until it becomes an issue for some actual library. I doubt any of the existing libraries have unhashable dtypes.
Activity
honno commentedon Oct 5, 2021
Working on this, thanks for the checklist. Will let you know if I have any questions.
honno commentedon Oct 11, 2021
RE hashability, worst case there's probably a hack we can do in
_array_moduleto wrap dtypes in hashable items (i.e. use dtype string names as hashes). This might be useful anyway so we can get consistentstr(dtype)output for things like error messages and pytest parameters.asmeurer commentedon Oct 13, 2021
That would mean we'd have to unpack the dtype object everywhere it is used, though. Let's not worry about this unless/until it becomes an issue for some actual library. I doubt any of the existing libraries have unhashable dtypes.