Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche
, '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

Add variable-length string support - #45

Closed
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support
Closed

Add variable-length string support#45
kgryte wants to merge 34 commits into
df-interchange-protocol-stagingfrom
variable-length-string-support

Conversation

@kgryte

@kgrytekgryte commented Jun 24, 2021

Copy link
Copy Markdown
Contributor

This PR

  • adds variable-length string support to the dataframe interchange protocol.
  • modifies the protocol to return a dictionary containing all possible data buffers, rather than separate methods for retrieving data, validity, and offsets buffers, as discussed in consortium meetings.
  • modifies the protocol to return both the buffers and their associated dtypes. This is necessary in order to interpret, e.g., the offsets and mask buffers.
  • updates source code documentation (punctuation, typos, and spelling).
  • manually copies string data to and from byte arrays, as it assumes that strings are stored as object dtype. The implementation will need to be updated to accommodate pandas' string extension dtype which is based on arrow. Currently, the string extension dtype is considered experimental and subject to change. The use of object dtype is still used as the default string dtype for backward compatiblity.
  • uses a byte array mask for indicating missing values in string data buffers. This can be updated to use a bit array for space efficiency, at the cost of additional code complexity.
  • currently does not handle the scenario where a non-pandas dataframe provided to __dataframe__ uses a bit array to indicate missing values.

@kgrytekgryte changed the title WIP: Add variable-length string supportAdd variable-length string supportJun 25, 2021
@kgryte
kgryte requested a review from rgommersJune 25, 2021 00:09
* Add a summary document for the dataframe interchange protocol
Summarizes the various discussions about and goals/non-goals
and requirements for the `__dataframe__` data interchange
protocol.
The intended audience for this document is Consortium members
and dataframe library maintainers who may want to support this
protocol.
The aim is to keep updating this till we have captured all
the requirements and answered all the FAQs, so we can actually
design the protocol after and verify it meets all our requirements.
Closesgh-29
* Process some review comments
* Process a few more review comments.
* Link to Release callback semantics in Arrow C Data Interface docs
* Add design requirements for column selection and df metadata
* Edit the nested/heterogeneous dtypes non-requirement
* Add requirements for chunking and memory layout description
Also address some smaller review comments.
* Add TBD notes on dataframe-array connection and from_dataframe
Also add more details on the Arrow C Data Interface.
* Address review comments
* Add details on implementation options
* Add details about the C implementation
* Add an image of the dataframe model and its memory layout.
* Add link to discussion on array-dataframe connection
* Some more updates for review comments
* Update table to indicate Arrow does support categoricals.
* Add section on dtype format strings
* Reflow some lines
* Add a requirement on semantic meaning of NaN/NaT, and timezone detail
* Textual tweak: say columns in a data frame are ordered
* Update requirements document for recent decisions/insights
Add a prototype of the dataframe interchange protocol
@rgommers

Copy link
Copy Markdown
Member

Thanks @kgryte! It may be useful to close this PR and resend it against main, now that the other PRs it needed are merged.

@rgommersrgommers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks very good, thanks @kgryte! I haven't tested it yet, but just reading through the code I have only a couple of comments.

Comment threadprotocol/pandas_implementation.py Outdated
Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py Outdated
@kgryte
kgryte changed the base branch from df-interchange-protocol-staging to mainJune 28, 2021 17:51
@kgryte
kgryte changed the base branch from main to df-interchange-protocol-stagingJune 28, 2021 17:51
@kgryte

Copy link
Copy Markdown
ContributorAuthor

@rgommers Will close this and submit against main in short order.

@jorisvandenbossche

Copy link
Copy Markdown
Member

Will close this and submit against main in short order.

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

Comment threadprotocol/dataframe_protocol_summary.md Outdated

@jorisvandenbosschejorisvandenbossche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the separate get_data_buffer and get_offsets methods might be a bit problematic. Typically you only want to do one pass over the data and create all buffers at once.

I know this is only a dummy implementation, and presumably the created buffers could be cached on the object so the separate methods don't calculate it twice. But might be useful to think about separate methods vs a single get_buffers() (with a specified order)

Comment threadprotocol/pandas_implementation.py
Comment threadprotocol/pandas_implementation.py
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
@kgryte

Copy link
Copy Markdown
ContributorAuthor

You can actually change the target branch by clicking the "Edit" button next to title, so then you don't need to close / open a new PR

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

@jorisvandenbossche

Copy link
Copy Markdown
Member

I initially tried doing that, but doing so added many unrelated changes and muddied this PR. :(

I think that either merging latest master in this branch or rebasing this branch on top of master should solve that

kgryte added a commit that referenced this pull request Jul 19, 2021
This is a fresh port of changes made in order to support variable
length strings in order to provide a cleaner merge.
@kgryte

Copy link
Copy Markdown
ContributorAuthor

Closing this PR out in favor of gh-47.

@kgrytekgryte closed this Jul 19, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requestinterchange-protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@kgryte@rgommers@jorisvandenbossche