feat: add Magika.get_content_type_info() public API method (fixes #826)#1318
feat: add Magika.get_content_type_info() public API method (fixes #826)#1318shivamtiwari3 wants to merge 2 commits intogoogle:mainfrom
Conversation
Root cause: the content type knowledge base (mime_type, group, description, extensions, is_text) was only accessible via the private _get_ct_info() method and the internal _cts_infos dict, with no supported public interface for external clients. Fix: expose a new public method Magika.get_content_type_info(label) that returns the ContentTypeInfo for any label in the knowledge base, raising MagikaError (rather than a bare KeyError) for unknown labels. This completes the content-type introspection API alongside the existing get_output_content_types() and get_model_content_types() methods.
There was a problem hiding this comment.
Code Review
This pull request introduces a new public API method get_content_type_info to expose content type metadata. The implementation is straightforward and includes basic tests. I've provided a few suggestions to improve the implementation by following Python best practices (EAFP), correcting the docstring, and enhancing test coverage to include error handling scenarios.
python/src/magika/magika.py
Outdated
| def get_content_type_info(self, label: ContentTypeLabel) -> ContentTypeInfo: | ||
| """Returns metadata for a given content type label. | ||
|
|
||
| This provides access to the content type knowledge base, exposing | ||
| structured metadata such as mime type, group, human-readable | ||
| description, common file extensions, and whether the type is | ||
| text-based. | ||
|
|
||
| Args: | ||
| label: The ContentTypeLabel to look up. | ||
|
|
||
| Returns: | ||
| The ContentTypeInfo for the given label. | ||
|
|
||
| Raises: | ||
| MagikaError: If the label is not present in the content type | ||
| knowledge base (e.g., ContentTypeLabel.UNDEFINED). | ||
| """ | ||
| if label not in self._cts_infos: | ||
| raise MagikaError( | ||
| f"Content type '{label}' is not in the content type knowledge base." | ||
| ) | ||
| return self._cts_infos[label] |
There was a problem hiding this comment.
This is a great addition to the public API. I have a couple of suggestions for improvement:
- Use EAFP pattern: For better adherence to Python idioms, consider using a
try...exceptblock (EAFP - "it's easier to ask for forgiveness than permission") instead of checking for the key's presence before accessing it. This is generally more performant if the key is expected to be present most of the time. - Fix docstring example: The example
ContentTypeLabel.UNDEFINEDin theRaisessection of the docstring appears to be incorrect, asundefinedis present incontent_types_kb.min.jsonand thus will not raise an error. To avoid confusion, it's best to remove the example.
Here's a suggested implementation incorporating these points:
def get_content_type_info(self, label: ContentTypeLabel) -> ContentTypeInfo:
"""Returns metadata for a given content type label.
This provides access to the content type knowledge base, exposing
structured metadata such as mime type, group, human-readable
description, common file extensions, and whether the type is
text-based.
Args:
label: The ContentTypeLabel to look up.
Returns:
The ContentTypeInfo for the given label.
Raises:
MagikaError: If the label is not present in the content type
knowledge base.
"""
try:
return self._cts_infos[label]
except KeyError:
raise MagikaError(
f"Content type '{label}' is not in the content type knowledge base."
) from None| def test_get_content_type_info() -> None: | ||
| """Test Magika.get_content_type_info() — fixes #826. | ||
|
|
||
| Verifies that the public API correctly exposes ContentTypeInfo metadata | ||
| (mime_type, group, description, extensions, is_text) for every | ||
| ContentTypeLabel that is part of the content type knowledge base. | ||
| """ | ||
| m = Magika() | ||
|
|
||
| # Every output content type must be queryable and return valid metadata. | ||
| for label in m.get_output_content_types(): | ||
| info = m.get_content_type_info(label) | ||
| assert isinstance(info, ContentTypeInfo) | ||
| assert info.label == label | ||
| assert isinstance(info.mime_type, str) and info.mime_type != "" | ||
| assert isinstance(info.group, str) and info.group != "" | ||
| assert isinstance(info.description, str) and info.description != "" | ||
| assert isinstance(info.extensions, list) | ||
| assert isinstance(info.is_text, bool) | ||
|
|
||
| # Spot-check a well-known content type's metadata. | ||
| pdf_info = m.get_content_type_info(ContentTypeLabel.PDF) | ||
| assert pdf_info.mime_type == "application/pdf" | ||
| assert pdf_info.group == "document" | ||
| assert pdf_info.is_text is False | ||
|
|
||
| # Text-based type should report is_text=True. | ||
| py_info = m.get_content_type_info(ContentTypeLabel.PYTHON) | ||
| assert py_info.is_text is True |
There was a problem hiding this comment.
This test covers the success paths well, but it's missing a check for the error condition. The get_content_type_info method is documented to raise a MagikaError for unknown labels. Please add a test case to verify this behavior.
def test_get_content_type_info() -> None:
"""Test Magika.get_content_type_info() — fixes #826.
Verifies that the public API correctly exposes ContentTypeInfo metadata
(mime_type, group, description, extensions, is_text) for every
ContentTypeLabel that is part of the content type knowledge base.
"""
from magika.types import MagikaError
m = Magika()
# Every output content type must be queryable and return valid metadata.
for label in m.get_output_content_types():
info = m.get_content_type_info(label)
assert isinstance(info, ContentTypeInfo)
assert info.label == label
assert isinstance(info.mime_type, str) and info.mime_type != ""
assert isinstance(info.group, str) and info.group != ""
assert isinstance(info.description, str) and info.description != ""
assert isinstance(info.extensions, list)
assert isinstance(info.is_text, bool)
# Spot-check a well-known content type's metadata.
pdf_info = m.get_content_type_info(ContentTypeLabel.PDF)
assert pdf_info.mime_type == "application/pdf"
assert pdf_info.group == "document"
assert pdf_info.is_text is False
# Text-based type should report is_text=True.
py_info = m.get_content_type_info(ContentTypeLabel.PYTHON)
assert py_info.is_text is True
# Verify that an unknown label raises MagikaError.
with pytest.raises(MagikaError, match="is not in the content type knowledge base"):
# Note: we use a string here because all ContentTypeLabel enum
# members are expected to be in the KB.
m.get_content_type_info("this_is_not_a_real_label")…r test Root cause: used LBYL (if key not in dict) instead of idiomatic Python EAFP. Fix: switch to try/except KeyError, remove incorrect UNDEFINED example from docstring, add a test asserting MagikaError for an unknown label.
|
Applied all three suggestions:
|
Summary
Fixes #826.
The content type knowledge base (mime type, group, description, extensions, is_text) was only accessible via the private
_get_ct_info()method and the internal_cts_infosdict. This PR exposes a publicget_content_type_info(label)method, completing the content-type introspection API alongside the existingget_output_content_types()andget_model_content_types()methods.Root Cause
Magika._get_ct_info()atmagika.py:344held all the content type metadata but was private and undocumented. External clients had no supported way to query mime type, group, description, or file extensions for a givenContentTypeLabel.Solution
Added
Magika.get_content_type_info(label: ContentTypeLabel) -> ContentTypeInfoinpython/src/magika/magika.py. The method:ContentTypeInfofor any label present in the knowledge baseMagikaError(rather than a bareKeyError) if the label is not in the KB, giving callers a clear error message_cts_infosdict — no new state introducedThe method fits naturally alongside
get_output_content_types()andget_model_content_types(): those methods answer "which labels exist?" whileget_content_type_info()answers "what does this label mean?"Testing
test_get_content_type_infoinpython/tests/test_magika_python_module.pythat:get_content_type_info()for every label returned byget_output_content_types()and asserts all fields are validContentTypeLabel.PDFfor exact expected values (application/pdf, groupdocument,is_text=False)ContentTypeLabel.PYTHONreturnsis_text=Trueuv run pytest tests -m "not slow"ruff check,ruff format, andmypypass cleanChecklist