aidenlab / straw

Extract data quickly from Juicebox via straw
MIT License
62 stars 36 forks source link

Bringing R C++ code up to date with the main C++ code #77

Closed cwenger closed 3 years ago

sa501428 commented 3 years ago

Apologies, I forgot to reply. Can we basically leave the python/C versions with the variable and just do the removal of those variables in the R wrapper version?

On Tue, Jun 22, 2021 at 9:54 AM Craig Wenger @.***> wrote:

@.**** commented on this pull request.

In C++/straw.cpp https://github.com/aidenlab/straw/pull/77#discussion_r656302722:

@@ -494,8 +494,8 @@ map<int32_t, indexEntry> readMatrixHttp(CURL *curl, int64_t myFilePosition, cons membuf sbuf(buffer, buffer + size); istream bufin(&sbuf);

  • int32_t c1 = readInt32FromFile(bufin);
  • int32_t c2 = readInt32FromFile(bufin);
  • readInt32FromFile(bufin);

@sa501428 https://github.com/sa501428 Does the latest commit satisfy your concern?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/aidenlab/straw/pull/77#discussion_r656302722, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRT23MF6K2FQZ2O6P7KCDDTUCP2JANCNFSM466GBOEA .

cwenger commented 3 years ago

Apologies, I forgot to reply. Can we basically leave the python/C versions with the variable and just do the removal of those variables in the R wrapper version? - Muhammad Saad Shamim http://mshamim.com/ On Tue, Jun 22, 2021 at 9:54 AM Craig Wenger @.> wrote: @*.** commented on this pull request. ------------------------------ In C++/straw.cpp <#77 (comment)>: > @@ -494,8 +494,8 @@ map<int32_t, indexEntry> readMatrixHttp(CURL *curl, int64_t myFilePosition, cons membuf sbuf(buffer, buffer + size); istream bufin(&sbuf); - int32_t c1 = readInt32FromFile(bufin); - int32_t c2 = readInt32FromFile(bufin); + readInt32FromFile(bufin); @sa501428 https://github.com/sa501428 Does the latest commit satisfy your concern? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#77 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRT23MF6K2FQZ2O6P7KCDDTUCP2JANCNFSM466GBOEA .

@sa501428 Absolutely; done in the latest commit.