-
Notifications
You must be signed in to change notification settings - Fork 36
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Replace Elite and EliteBatch with dicts (#397)
## Description <!-- Provide a brief description of the PR's purpose here. --> This PR removes the Elite and EliteBatch namedtuples from the public API; instead, we create an Elite and EliteBatch namedtuple on the fly in each archive. This allows us to support custom field names in each namedtuple in the future. In creating this PR, I was considering whether to create a custom namedtuple for each archive (when the archive is constructed, similar to how pandas has itertuples), or to use a dict. These were the pros and cons I came up with of dicts over such namedtuples: Pros: - This is clearly backwards-incompatible, so users will know something has broken. The old tuple unpacking behavior will definitely not work here, and calling the attributes also will not work. - Dicts are less finicky than namedtuples, in that there are no attributes to manage. - Dicts are already a common data structure; people already know how to get the keys etc - It is easier to handle retrieving just a couple of fields. In such a case, we can just add the required keys to the dict. In contrast, we would have to set some fields to None in a namedtuple - We no longer will have a name conflict with the index method of namedtuples Cons: - The old unpacking logic will no longer work - Getting attributes will no longer work - Harder to tell which things are batch because it’s not in the name, although I think it’s usually clear from the context ## TODO <!-- Notable points that this PR has either accomplished or will accomplish. --> - [x] Replace all usages - [x] Double check for usage of Elite and EliteBatch ## Questions <!-- Any concerns or points of confusion? --> ## Status - [x] I have read the guidelines in [CONTRIBUTING.md](https://github.com/icaros-usc/pyribs/blob/master/CONTRIBUTING.md) - [x] I have formatted my code using `yapf` - [x] I have tested my code by running `pytest` - [x] I have linted my code with `pylint` - [x] I have added a one-line description of my change to the changelog in `HISTORY.md` - [x] This PR is ready to go
- Loading branch information
Showing
18 changed files
with
238 additions
and
300 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.