syun64 commented on PR #394: URL: https://github.com/apache/iceberg-python/pull/394#issuecomment-1971358205
> Hey @syun64 Thanks for adding the yaml, that looks neat. What's your gist of Griffe? It looks like there are already some false positives. Thank you for all the feedback @Fokko ! My general impression with griffe is that it's working pretty well, and after carefully reviewing and adding even more breaking changes we've introduced since the last release, I feel convinced that we should include some implementation of this tool. There are some edge cases that I thought were worth summarizing: 1. Cython Class interface inference isn't perfect. As I noted in this [comment](https://github.com/apache/iceberg-python/pull/394#discussion_r1506659256), either the **.so** or **.pyi** files are being used. And when .so file is used to interprete the interface, it returns **None** return types for all of its members: ``` >>> griffe.load("pyiceberg")['avro']['decoder_fast']['CythonBinaryDecoder'].relative_filepath PosixPath('pyiceberg/avro/decoder_fast.cpython-38-x86_64-linux-gnu.so') >>> griffe.load("pyiceberg")['avro']['decoder_fast']['CythonBinaryDecoder']['tell'].returns >>> griffe.load_git("pyiceberg")['avro']['decoder_fast']['CythonBinaryDecoder'].relative_filepath PosixPath('/tmp/griffe-worktree-iceberg-python-HEAD-nu9f66g2/griffe_HEAD/pyiceberg/avro/decoder_fast.pyi') >>> griffe.load_git("pyiceberg")['avro']['decoder_fast']['CythonBinaryDecoder']['tell'].returns ExprName(name='int', parent=Class('CythonBinaryDecoder', 20, 73)) ``` 2. **Import removal** is also considered as a breaking change because it's a public function/Class that was available on the module, that's been removed. Outside of these two edge cases, I think all the exclusions listed are very accurate, and I think the test will require the contributors to think twice about making a breaking change, or to document an intended breaking change into the yaml file so we developers are able to trace down breaking changes a lot easier. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
