Skip to content

docs(#1458): fix FloatArrayCodec get_dtype example (<hash>, not <hash@>)#192

Merged
dimitri-yatsenko merged 1 commit into
mainfrom
docs/fix-hash-dtype-example
Jul 1, 2026
Merged

docs(#1458): fix FloatArrayCodec get_dtype example (<hash>, not <hash@>)#192
dimitri-yatsenko merged 1 commit into
mainfrom
docs/fix-hash-dtype-example

Conversation

@dimitri-yatsenko

Copy link
Copy Markdown
Member

Follow-up to #188.

The FloatArrayCodec.get_dtype example in spark-adapter.md returned "<hash@>" for the store branch. Codecs return the base dtype "<hash>" — the framework composes the @ store modifier at declaration time, not the codec. This matches the built-in BlobCodec.get_dtype, which returns "<hash>".

     def get_dtype(self, is_store: bool) -> str:
-        return "longblob" if not is_store else "<hash@>"
+        return "longblob" if not is_store else "<hash>"

Flagged by @MilagrosMarin in her review of #188 ("plugin authors copy-pasting the example may see divergent behavior").

Note: the two cosmetic residuals she also noted (stale renderable.md/renderable-codecs.md filename table and the "stays draft" framing) lived in the #188 PR description, not in the merged files, so nothing to fix in-tree for those.

The get_dtype example returned "<hash@>" for the store branch, but codecs
return the base dtype "<hash>" — the framework composes the @ store modifier
at declaration time. Matches built-in BlobCodec.get_dtype. Flagged by
@MilagrosMarin in review of #188.

@MilagrosMarin MilagrosMarin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified. Matches BlobCodec.get_dtype on master (src/datajoint/builtin_codecs/blob.py: return "<hash>" if is_store else "bytes"). One-line fix for the exact concern I raised on #188. Approving.

@dimitri-yatsenko dimitri-yatsenko merged commit 0e8a4f4 into main Jul 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants