ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871

Closed
wesm wants to merge 2 commits into
apache:masterfrom
wesm:ARROW-2119
Closed

ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom
wesm:ARROW-2119

Conversation

@wesm

@wesmwesm commented Mar 12, 2019

Copy link
Copy Markdown
Member

I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.

This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also

@wesm

wesm commented Mar 12, 2019

Copy link
Copy Markdown
MemberAuthor

See integration test results:

https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2

It looks like only C++ can consume its own input. The other 8 entries of the matrix fail

@wesm

wesm commented Mar 13, 2019

Copy link
Copy Markdown
MemberAuthor

Hm, well this is a bit concerning. Failure in the integration tests does not fail the build

@fsaintjacques

fsaintjacques commented Mar 13, 2019

Copy link
Copy Markdown
Contributor

Rebase and try again

@emkornfield

emkornfield commented Mar 14, 2019

Copy link
Copy Markdown
Contributor

@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).

@wesm

wesm commented Mar 14, 2019

Copy link
Copy Markdown
MemberAuthor

yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches

@emkornfield

emkornfield commented May 16, 2019

Copy link
Copy Markdown
Contributor

@wesm were you going to follow up on the ML about this? Want me to?

@wesm

wesm commented May 20, 2019

Copy link
Copy Markdown
MemberAuthor

@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least

@trxcllnttrxcllnt mentioned this pull request May 22, 2019
Closed
@wesmwesm changed the title WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm

wesm commented May 23, 2019

Copy link
Copy Markdown
MemberAuthor

I rebased and set the integration test to only run for C++

@wesm

wesm commented May 23, 2019

Copy link
Copy Markdown
MemberAuthor

+1

@wesmwesm closed this in b3a4e95May 23, 2019
@wesm
wesm deleted the ARROW-2119 branch May 23, 2019 15:46
wesm pushed a commit that referenced this pull request May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches
This was referenced May 23, 2019
Closed
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches - #3871
Closed
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Closed
ARROW-2119: [IntegrationTest] Add test case with a stream having no record batches#3871
wesm wants to merge 2 commits into
apache:masterfrom 
wesm:ARROW-2119
Conversation
@wesm
@wesmwesm
commented
Mar 12, 2019

Copy link


Copy Markdown

Member
I think it is not a bad idea to not fail on reading and writing streams that have no record batches, rather than raising an error.
This would require code changes in Java and JS at least, so I will need help from others if this is thought to be a good idea. Might be good to add some unit tests around this also
@wesm
wesm
commented
Mar 12, 2019

Copy link


Copy Markdown

MemberAuthor
See integration test results:
https://gist.github.com/wesm/df111020b498f3e7b261b8667aced4e2
It looks like only C++ can consume its own input. The other 8 entries of the matrix fail
@wesm
wesm
commented
Mar 13, 2019

Copy link


Copy Markdown

MemberAuthor
Hm, well this is a bit concerning. Failure in the integration tests does not fail the build
@fsaintjacques
fsaintjacques
commented
Mar 13, 2019

Copy link


Copy Markdown

Contributor
Rebase and try again
@emkornfield
emkornfield
commented
Mar 14, 2019

Copy link


Copy Markdown

Contributor
@wesm I can take up the Java side of things in the 0.14 (sorry a bit swamped at the moment) release time frame. Does it pay to discuss this on the ML to be sure there is consensus? (Apologies if I missed the thread).
@wesm
wesm
commented
Mar 14, 2019

Copy link


Copy Markdown

MemberAuthor
yeah I think it would make sense to ensure there is agreement about what should occur with a stream with no batches
@emkornfield
emkornfield
commented
May 16, 2019

Copy link


Copy Markdown

Contributor
@wesm were you going to follow up on the ML about this? Want me to?
@wesm
wesm
commented
May 20, 2019

Copy link


Copy Markdown

MemberAuthor
@emkornfield I guess I dropped the ball. Do you want to ping the list about it? Then we can try to fix the Java and C++ implementations at least
@trxcllnttrxcllnt
mentioned this pull request
May 22, 2019
 Closed

@wesmwesm
changed the title
WIP ARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesARROW-2119: [IntegrationTest] Add test case with a stream having no record batchesMay 23, 2019
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
I rebased and set the integration test to only run for C++
@wesm
wesm
commented
May 23, 2019

Copy link


Copy Markdown

MemberAuthor
+1
@wesmwesm
closed this
in
b3a4e95May 23, 2019
@wesm
wesm
deleted the

ARROW-2119

branch
May 23, 2019 15:46
wesm pushed a commit
that referenced
this pull request
May 31, 2019
@trxcllnt@wesm
ARROW-5396: [JS] Support files and streams with no record batches
17e0198
Re: #3871, [ARROW-2119](https://issues.apache.org/jira/browse/ARROW-2119), and closes [ARROW-5396](https://issues.apache.org/jira/browse/ARROW-5396).
This PR updates the JS Readers and Writers to support files and streams with no RecordBatches. The approach here is two-fold:
1. If the Readers' source message stream terminates after reading the Schema message, the Reader will yield a dummy zero-length RecordBatch with the schema.
2. The Writer always writes the schema for any RecordBatch, but skips writing the RecordBatch field metadata if it's empty.
This is necessary because the reader and writer don't know about each other when they're communicating via the Node and DOM stream i/o primitives; they only know about the values pushed through the streams. Since the RecordBatchReader and Writer don't yield the Schema message as a standalone value, we pump the stream with a zero-length RecordBatch that contains the schema instead.
Author: ptaylor <paul.e.taylor@me.com>
Author: Wes McKinney <wesm+git@apache.org>
Closes#4373 from trxcllnt/js/fix-no-record-batches and squashes the following commits:
c860696 <Wes McKinney> Run no-batches integration test for JS also
86d192d <ptaylor> define an _InternalEmptyRecordBatch class to signal that the reader source stream has no RecordBatches
193b08d <ptaylor> ensure reader and writer support the case where a stream or file has a schema but no recordbatches

This was referenced May 23, 2019

Closed


Closed


Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

3 participants

@wesm@fsaintjacques@emkornfield