Remove _batch from archive add() parameter names
#422
Merged
Conversation
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
8 tasks
btjanaka
added a commit
that referenced
this pull request
Nov 17, 2023
## Description <!-- Provide a brief description of the PR's purpose here. --> Continuation of #422. Note that I still use measures_batch and other batch names inside of methods themselves when it is necessary to have clarity. ## TODO <!-- Notable points that this PR has either accomplished or will accomplish. --> - [x] Rename measures_batch to measures in `index_of` - [x] Rename measures_batch to measures in `retrieve` ## 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
8 tasks
btjanaka
added a commit
that referenced
this pull request
Nov 17, 2023
## Description <!-- Provide a brief description of the PR's purpose here. --> Similar to #422, this removes `_batch` from parameter names in the scheduler. Even here, in a part of the API that is frequently used, I still anticipate that it will not affect users much, since most will pass in arguments positionally rather than as keywords. ## TODO <!-- Notable points that this PR has either accomplished or will accomplish. --> - [x] Rename in both schedulers - [x] Check tests ## 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
10 tasks
btjanaka
added a commit
that referenced
this pull request
Nov 27, 2023
## Description <!-- Provide a brief description of the PR's purpose here. --> Similar to #422 and #425, this PR removes the `_batch` suffix from parameter names in emitter tell and tell_dqd methods. I have left status_batch and value_batch alone for the time being as they are not data that are passed to the archive. ## TODO <!-- Notable points that this PR has either accomplished or will accomplish. --> - [x] Switch names in EmitterBase - [x] Switch names in other emitters - [x] Switch to validate_batch for argument validation - [x] Remove validata_batch_args function ## 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Removes the
_batchsuffix from parameter names for add(), e.g.,solution_batchjust becomessolution. This change helps to make way for #421, where we support custom fields. With custom fields, it makes sense to just have the field name in the argument, e.g.,myfield, rather thanmyfield_batch. Thus, this PR makes the current argument names consistent with this usage.Furthermore, the names with
_batchare rather verbose, as it is usually clear from context whether the data is batch or singular (i.e., our methods mostly take in batch data, and when they need single data, they are named with the_singlesuffix, e.g.,add_single).This change is not as likely to affect users, since most only deal with the scheduler and do not call the archive directly; furthermore, most who do use add will only pass in their arguments positionally -- our schedulers currently use positional arguments.
TODO
Questions
Status
CONTRIBUTING.md
yapfpytestpylintHISTORY.md