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

dont pirate convert: fix https://github.com/mauro3/Parameters.jl/issues/73 #20

Merged
merged 4 commits into from
Sep 5, 2018

Conversation

piever
Copy link
Collaborator

@piever piever commented Aug 21, 2018

No description provided.

@codecov-io
Copy link

codecov-io commented Aug 21, 2018

Codecov Report

Merging #20 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #20   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           6      6           
  Lines          52     52           
=====================================
  Hits           52     52
Impacted Files Coverage Δ
src/shiftedarray.jl 100% <ø> (ø) ⬆️
src/reduce.jl 100% <100%> (ø) ⬆️
src/offset.jl 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb5b8ad...11a911a. Read the comment docs.

@cstjean
Copy link

cstjean commented Sep 4, 2018

I've been using this branch, and it fixed my issues. Any reason not to merge?

@piever
Copy link
Collaborator Author

piever commented Sep 4, 2018

Somehow I'm not sure that ShiftedArrays.convert(Array, v) is the best interface to convert a vector of N dimensional ShiftedArray into a N+1 dimensional Array. Do you have any suggestions on the API?

@cstjean
Copy link

cstjean commented Sep 4, 2018

array_from(vec_of_shifted_arrays)? I agree that convert is not super appropriate.

@piever
Copy link
Collaborator Author

piever commented Sep 5, 2018

You're right that this is not quite a conversion. I'll add some dedicated helper functions name and merge.

@piever piever merged commit bfb6622 into master Sep 5, 2018
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.

3 participants