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

NArray compatibility #361

Merged
merged 4 commits into from
May 11, 2015

Conversation

blackwinter
Copy link

When NArray is installed alongside NMatrix, require 'nmatrix' will load NArray's lib/nmatrix.rb file. By moving all relevant code from NMatrix's lib/nmatrix.rb to lib/nmatrix/nmatrix.rb, users can require 'nmatrix/nmatrix' to load NMatrix and thus bypass NArray. (Related issues: #6, #172 and masa16/narray#39; also SciRuby/rb-gsl#14)

  • The order in which code was loaded previously has been preserved.
  • I have cleaned up the newly added requires in an extra commit.
  • I have also included documentation changes in extra commits. Feel free to ignore.

Please note that I didn't go the extra mile and set up a test environment locally. I trust that it continues to work, based on the nature of the changes, and let Travis do the rest.

…rix.rb.

When NArray is installed alongside NMatrix, `require 'nmatrix'` will load NArray's `lib/nmatrix.rb` file. By moving all relevant code from NMatrix's `lib/nmatrix.rb` to `lib/nmatrix/nmatrix.rb`, users can `require 'nmatrix/nmatrix'` to load NMatrix and thus bypass NArray.

Note: The order in which code was loaded previously has been preserved.
* Changed `require` to `require_relative`.
* Inserted leading `.` to match existing style.
* Dropped duplicates `monkeys` and `shortcuts.rb`.

Note: The load mechanism for `nmatrix.so` has not been touched as there seem to be some hacks going on that I didn't want to mess with.
translunar added a commit that referenced this pull request May 11, 2015
@translunar translunar merged commit a49350b into SciRuby:master May 11, 2015
@translunar
Copy link
Member

This is beautiful. Thank you so much.

@blackwinter blackwinter deleted the narray-compatibility branch May 12, 2015 13:20
@gbuesing
Copy link

This works if you know about it, but any project that does a bare require 'nmatrix' as shown in the README can still run into issues -- if the narray gem is later installed or added to a project, you could suddenly get a load error, or (even more confusingly) be working with a completely different NMatrix class.

Given this, I'd suggest standardizing on one preferred way to require in the docs/readme, and then print a warning when doing require 'nmatrix' indicating the preferred way to require: require 'nmatrix/nmatrix'.

Or better yet, enable doing require 'sciruby/nmatrix' (which could just require 'nmatrix/nmatrix') and standardize with that.

agisga added a commit to agisga/mixed_models that referenced this pull request Aug 30, 2015
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