Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consolidate definitions to use TensorProtocol #40

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

philipce
Copy link
Owner

Description of Changes

Prior to the creation of the TensorProtocol--to which Vector, Matrix, and Tensor all conform--various functions were implemented and overloaded to accept all 3. This duplication is not ideal so these changes consolidate the function definitions into a genericized version.

Resolves

Todos

Author:

  • Write up description and link related tickets
  • Ensure your changes include good test coverage
  • Check out our Style Guide
  • Add an approved reviewer
  • Update the ticket status in the relevant tracker (optional)
  • Add benchmark scripts and request benchmarking from reviewer (optional)
  • Respond to requests for changes from the reviewer as needed

Reviewer:

  • Review changes for style, correctness, performance and request changes as necessary.
  • Update the Status document
  • Update the Benchmarks document (optional)
  • Update the ticket status in the relevant tracker
  • Merge into master and change base of any dependent branches
  • Delete this branch

@philipce philipce self-assigned this Jul 25, 2018
@philipce philipce added the WIP label Jul 25, 2018
loseth
loseth previously approved these changes Jul 30, 2018
Copy link
Collaborator

@loseth loseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both tests and some example code passed.

@philipce philipce dismissed loseth’s stale review July 30, 2018 18:46

Not yet finished

@philipce
Copy link
Owner Author

Thanks for looking at this! There are a handful of other functions that need this same change that I'll just put on this PR. Usually I use the WIP (work in progress) label to indicate it's not ready, and stick 'reviewable' on it when it is

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants