Conversation
|
Note: pyi files must be updated |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: The pull request introduces a significant enhancement by adding wrapper support for file streams, enabling the use of Standard_IStream and Standard_OStream with Python strings. This enhancement facilitates object serialization and import/export operations, such as handling STEP or GLTF files, through a more Pythonic interface. The changes span across multiple test files, adding new unit tests to ensure the functionality works as expected and modifying existing tests to align with the new approach.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Consider adding more comprehensive documentation within the code to explain the usage and benefits of the new wrapper support for file streams. This could help future developers and users understand the feature more quickly.
- Evaluate the performance implications of the new wrapper support, especially for large files or in high-throughput scenarios, to ensure that the enhancement does not introduce any significant overhead.
- Explore the possibility of extending this wrapper support to other file types or serialization formats beyond STEP or GLTF, to further increase the utility of the library.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
src/SWIG_files/wrapper/BOPAlgo.pyi
Outdated
| def Allocator(self) -> NCollection_BaseAllocator: ... | ||
| def Clear(self) -> None: ... | ||
| def ClearWarnings(self) -> None: ... | ||
| def DumpErrors(self) -> False: ... |
There was a problem hiding this comment.
🚨 security (llm): It seems like the return type for DumpErrors should be bool instead of False. Please verify if this is a typo or intentional design.
There was a problem hiding this comment.
Right, this has to be fixed
There was a problem hiding this comment.
PR Type: Enhancement
PR Summary: This pull request introduces the capability to handle file streams as strings within the Python interface of a C++ library. It focuses on enabling the serialization and deserialization of objects, as well as the import and export of files, through the use of wrapper functions that convert C++ stream handling methods to Python string operations. This enhancement leverages SWIG templates to bridge the gap between C++ streams and Python strings, facilitating operations like reading from or writing to files in a more Pythonic manner.
Decision: Comment
📝 Type: 'Enhancement' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
- Unsupported files: the diff contains files that Sourcery does not currently support during reviews.
General suggestions:
- Ensure comprehensive documentation is provided for the new wrapper functions to aid users in understanding their usage and limitations.
- Consider adding more examples or use cases in the documentation or in the test suite to demonstrate the practical applications of these enhancements.
- Review the consistency of naming conventions for the new methods to ensure they align with existing naming patterns and are intuitive for users.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
tpaviot
force-pushed
the
review/iostream
branch
from
181d011 to
423c2ab
Compare
1 participant
Superseeds #1299
This branch adds support for
Standard_IStreamandStandard_OStreamas python stringsc++ methods:
ReadStream(std::istream & theIStream)are wrapped as
and
WriteStream(std::ostream & theOStream)can be used in python with:
This is used for object serialization (as an ascii string
DumporDumpJsonInitFromJson) as well as import export (STEP or GLTF files).This is achieved using the set of
IOStream.iswig templates, and a change over the whole interface files generated from tpaviot/pythonocc-generator@d168c34