Update make_data_bin() to return DataBin proper #12219
Conversation
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 8923416015Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
Yes, I plan to include the methods added in #12129. I'm hoping to merge 12129 first because it doesn't have any of the concerns that are mentioned in the summary of this PR. |
| shape = ",".join(map(str, cls._SHAPE)) | ||
| return f"{name}<{shape}>" | ||
| class DataBin(metaclass=DataBinMeta): | ||
| """Namespace for storing data. |
There was a problem hiding this comment.
Should this inherit from Mapping now? Also its now both a namespace and a mapping container (just needs an iter to complete the mapping abc i think)
There was a problem hiding this comment.
The iter already exists from a different PR. I'd like to control our own namespace, so I think I'm happy to be a mapping only via protocol.
I checked all of our repos and it's not used by anything
|
Thank you for overhauling data bin. It looks good overall. Could you write a reno? |
|
I noticed that we can simplify the code slightly. I wrote a suggestion ihincks#2. |
Use views instead of sequences
sbrandhsn
added
the
Changelog: New Feature
* Update make_data_bin() to return DataBin proper * Remove metaclass I checked all of our repos and it's not used by anything * fix implementations * remove make_data_bin usage * make the DataBin follow the Shaped protocol * add it back to containers module (removed by accident) * get implemntatinos to assign shape * improve the repr * add reno * fix reno formatting * improve error string * use views instead of sequences * fix tests --------- Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com>
Summary
This PR updates
DataBinwith the intention of making it easier to construct directly, avoiding themake_data_binclass.I have concerns that this breaks the class because it drops support for positional arguments in the constructor, but am opening up the PR anyways. There's also no way to properly infer shape at instance construction time, but there is (as previously implemented) at dynamic type construction time.
Details and comments
This PR attempts to keep legacy support for a bunch of behaviour that is technically private, but that has become widespread due to lack of a way to achieve certain basic tasks (like find namespace names) in a simple way.