Skip to content

Conversation

@pcmoritz
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #3392 into master will increase coverage by 0.99%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3392      +/-   ##
==========================================
+ Coverage   88.46%   89.45%   +0.99%     
==========================================
  Files         631      502     -129     
  Lines       79288    70318    -8970     
  Branches     1069        0    -1069     
==========================================
- Hits        70140    62905    -7235     
+ Misses       9037     7413    -1624     
+ Partials      111        0     -111
Impacted Files Coverage Δ
cpp/src/plasma/io.cc 74.21% <ø> (ø) ⬆️
cpp/src/plasma/common.h 100% <ø> (ø) ⬆️
cpp/src/plasma/common.cc 93.33% <0%> (-0.15%) ⬇️
cpp/src/arrow/util/bpacking.h 99.84% <0%> (-0.01%) ⬇️
go/arrow/math/uint64_amd64.go
go/arrow/memory/memory_avx2_amd64.go
js/src/enum.ts
go/arrow/array/builder.go
js/src/Arrow.node.ts
js/src/schema.ts
... and 124 more

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 5598d2f...3b5473a. Read the comment docs.

@robertnishihara robertnishihara deleted the plasma-dcheck branch January 13, 2019 23:51
pcmoritz added a commit to pcmoritz/ray-1 that referenced this pull request Jan 14, 2019
guoyuhong pushed a commit to ray-project/ray that referenced this pull request Jan 14, 2019
* update arrow to include apache/arrow#3392

* add appropriate includes

* update
#include "arrow/adapters/tensorflow/convert.h"
#include "arrow/api.h"
#include "arrow/io/api.h"
#include "arrow/util/logging.h"
Copy link
Member

Choose a reason for hiding this comment

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

@pcmoritz Would you know why plasma_op.cc includes Tensorflow's logging.h above? It produces a collision in the DCHECK macros.

Copy link
Member

Choose a reason for hiding this comment

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

where is the DCHECK collision occurring? Are we clobbering their macro or vice versa? We should use ARROW_CHECK (or add ARROW_DCHECK_* aliases) so we can use these in an environment where there is ambiguity. We already disallow DCHECK in public headers

Copy link
Member

Choose a reason for hiding this comment

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

See line 22 above:

#include "tensorflow/core/platform/logging.h"

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.

4 participants