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

Improved vector tests; removed redundant benchmarks #127

Merged
merged 2 commits into from
Sep 27, 2019

Conversation

regexident
Copy link
Collaborator

Vector's operations shallowly wrap those from "Arithmetic", so there's really no point in benchmarking them, when we already have benchmarks in place for the latter.

Vector tests now also moved from explicit expected values via literals to ad-hoc un-optimized calculations of the operation.

Copy link
Collaborator

@mattt mattt left a comment

Choose a reason for hiding this comment

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

Nice! I especially like the change from literals to constructors.

Only thing I bumped on was the use of explicit Swift. namespacing for the zip function (and maybe also Surge.dist). Are these necessary to satisfy compiler type checking, or stylistic (apologies if I'm missing an obvious collision somewhere).

@regexident
Copy link
Collaborator Author

Swift.: I prefer to stay on the safe side of things, given Swift's lack of coherence/orphan checks, hence I got used to adding Swift. whenever I use a top-level function from the stlib.

I removed them in c86b964. :)

@regexident regexident merged commit 3f2f221 into master Sep 27, 2019
@regexident regexident deleted the improve-vector-tests branch September 27, 2019 15:12
@mattt
Copy link
Collaborator

mattt commented Sep 27, 2019

@regexident I definitely get where you're coming from — I remember getting bitten by that in early versions of Swift (IIRC in relation to Darwin functions). But I think you made the right choice with 53b46a6; an explicit but unnecessary Swift. could cause an observer to think that the current module (or an imported one) defines a function with the same name. So thanks for that!

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

Successfully merging this pull request may close these issues.

2 participants