Conversation
Clarified usage of context managers with Builder, Reader, and Signer classes.
…' into mathern/context
…' into mathern/context
* fix: Fragment API
* fix: Managed resources docs * fix: Docs * fix: Docs * fix: Docs * fix: Docs * fix: Update docs --------- Co-authored-by: Tania Mathern <tania.mathern@gmail.comn>
* fix: New API * fix: Review feedback on c2pa-rs PR * fix: Docs * v0.77.1 of c2pa-rs is released * Update c2pa version to v0.77.1 * fix: Merge conflict mistake --------- Co-authored-by: Tania Mathern <tania.mathern@gmail.comn>
| settings = Settings.from_dict({"builder": {"thumbnail": {"enabled": False}}}) | ||
|
|
||
| # Merge additional configuration | ||
| settings.update({"verify": {"remote_manifest_fetch": True}}) |
There was a problem hiding this comment.
Does this overwrite if the property already exists? May be helpful to include an example that shows that it does or doesn't.
There was a problem hiding this comment.
Yes, and examples are included (tests) + it's in the docs too, but in another PR with all the docs (that one is code only/mostly).
| @property | ||
| @abstractmethod | ||
| def is_valid(self) -> bool: ... | ||
|
|
||
| def __init__(self, file_like_stream): | ||
| """Initialize a new Stream wrapper around a file-like object | ||
| (or an in-memory stream). | ||
| @property | ||
| @abstractmethod | ||
| def execution_context(self): ... |
There was a problem hiding this comment.
I don't know if this is standard for Python, but I typically like to see function descriptions (I suppose docstrings in this case) in the abstract base classes to describe what the abstract methods should do at a high-level, so that subclasses have some guidance on their implementations.
I see that we have some docstrings for these in the ManagedResource and Context classes -- could they be copied here?
| """Fluent builder for Context. | ||
|
|
||
| Use Context.builder() to create an instance. | ||
| """ |
There was a problem hiding this comment.
A bit off-topic, but what are the motivations behind implementing this in the fluent design pattern? I don't have a strong opinion about it, but just wondering if there was a particular driver behind it.
Changes in this pull request
Main changes are in
src/c2pa/c2pa.py.Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.