Extend file stream#12
Conversation
📝 WalkthroughWalkthroughAdds a new file-options module and option classes, exposes them via public packages, replaces plain dict file-format mappings with AliasDict (including aliases like .yaml→.yml), adds FileStream option normalization, updates CI lint checks, and restructures/expands tests and test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FileStream
participant Options as FileOpts / FormatOpts
participant AliasDictConfig as AliasDictConfig
participant FS as Filesystem
Caller->>FileStream: save(path, data, options)
FileStream->>Options: receive options (obj|dict|None)
Options-->>FileStream: (if object) to_dict via Mappable
FileStream->>AliasDictConfig: lookup config by path.suffix (handles aliases)
AliasDictConfig-->>FileStream: format config (delimiter, handler, etc.)
FileStream->>FS: open + write using normalized options and format config
FS-->>FileStream: write result / error
FileStream-->>Caller: success / raise
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyrio/streams/file_stream.py (1)
170-177: Bug: Wrong config mapping used for write operation.Line 172 checks
suffix in MAPPING_READ_CONFIGbut this is within thesave()method which should useMAPPING_WRITE_CONFIG. This could cause incorrect behavior when writing files with formats that have different read/write configurations.🐛 Proposed fix
if (suffix := path.suffix) in DSV_TYPES: return self._write_dsv(path, tmp_path, f_open_options, f_write_options, null_handler) - elif suffix in MAPPING_READ_CONFIG: + elif suffix in MAPPING_WRITE_CONFIG: return self._write_mapping( path, tmp_path, f_open_options, f_write_options, null_handler, **kwargs )
🤖 Fix all issues with AI agents
In `@pyrio/streams/file_stream.py`:
- Around line 38-39: In save() in file_stream.py, the suffix check is using the
wrong mapping: replace the condition that tests "suffix in MAPPING_READ_CONFIG"
with "suffix in MAPPING_WRITE_CONFIG" so the write-path uses the write
configuration; ensure the check references the local suffix variable and then
proceeds to use MAPPING_WRITE_CONFIG to look up writer behavior (keep the rest
of save() unchanged).
In `@tests/test_file_stream.py`:
- Around line 442-452: The test currently calls _raise(IOError(...)) during
argument evaluation so the exception fires before FileStream.save runs; instead
defer invocation by passing a callable (e.g., null_handler=lambda:
_raise(IOError("Ooops Mr White...")) or null_handler=functools.partial(_raise,
IOError("Ooops Mr White..."))) so the exception is raised inside
FileStream.save's null handling path; update the test_update_fails to pass the
callable rather than calling _raise directly.
🧹 Nitpick comments (2)
pyrio/utils/file_options.py (1)
1-11: Consider removing ABC inheritance since there are no abstract methods.
Mappableinherits fromABCbut defines no@abstractmethod. TheABCbase class is typically used to enforce that subclasses implement certain methods. SinceMappableprovides concrete implementations ofto_dict()and__repr__(), it could be a regular base class.♻️ Suggested simplification
-from abc import ABC - - -class Mappable(ABC): +class Mappable: def to_dict(self): """Returns dict with only non-None values""" return {attr: val for attr, val in vars(self).items() if val is not None}tests/test_stream.py (1)
882-910: Consider moving standalone tests into the TestStream class for consistency.The
test_hackerrankandtest_leetcodefunctions remain as standalone tests while the rest are organized inTestStream. For consistency, these could be moved into the class or into a separateTestExamplesclass.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyrio/utils/file_options.py`:
- Around line 215-232: The static method pretty in class XmlWriteOpts conflicts
with the instance attribute self.pretty set in __init__, causing instance.pretty
to reference the method instead of the boolean; rename the static factory method
(e.g., to pretty_opts or pretty_options) or make it a `@classmethod` with a
different name so it no longer shadows the instance attribute, update its
docstring accordingly and ensure it still returns XmlWriteOpts(pretty=True,
indent=indent).
| class XmlWriteOpts(Mappable): | ||
| """Options for writing XML files""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| pretty=None, | ||
| indent=None, | ||
| short_empty_elements=None, | ||
| ): | ||
| self.pretty = pretty | ||
| self.indent = indent | ||
| self.short_empty_elements = short_empty_elements | ||
|
|
||
| @staticmethod | ||
| def pretty(indent=4): | ||
| """Creates XmlWriteOptions for pretty-printed output""" | ||
| return XmlWriteOpts(pretty=True, indent=indent) | ||
|
|
There was a problem hiding this comment.
Static method pretty shadows instance attribute self.pretty.
The static method pretty(indent=4) at line 228 shadows the instance attribute self.pretty set in __init__ at line 224. After instantiation, accessing instance.pretty will return the static method instead of the boolean value.
Consider renaming the static method to avoid the conflict:
🔧 Suggested fix
`@staticmethod`
- def pretty(indent=4):
+ def pretty_print(indent=4):
"""Creates XmlWriteOptions for pretty-printed output"""
return XmlWriteOpts(pretty=True, indent=indent)🤖 Prompt for AI Agents
In `@pyrio/utils/file_options.py` around lines 215 - 232, The static method pretty
in class XmlWriteOpts conflicts with the instance attribute self.pretty set in
__init__, causing instance.pretty to reference the method instead of the
boolean; rename the static factory method (e.g., to pretty_opts or
pretty_options) or make it a `@classmethod` with a different name so it no longer
shadows the instance attribute, update its docstring accordingly and ensure it
still returns XmlWriteOpts(pretty=True, indent=indent).
1 participant
Summary by CodeRabbit
New Features
.yaml/.ymlalias support.Chores
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.