fix: opinionated setup and clean fn fix#240
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Added opinionated development setup instructions to the README.md, introducing uv as the recommended installation method with specific version pinning (Python 3.9.19).
- Added prescriptive setup block in README.md with explicit version requirements while maintaining existing flexible setup options
- Introduced
uvas preferred package manager for more reliable dependency management - Aligned setup instructions with minimum Python version requirement (3.9+) from setup.py
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
pauldambra
changed the title
|
wat |
pauldambra
changed the title
pauldambra
changed the title
There was a problem hiding this comment.
might want to go commit by commit
i tidied this up as well as fixing some assertions
| def test_clean_pydantic_like_class(self) -> None: | ||
| class Dummy: | ||
| def model_dump(self, required_param): | ||
| pass | ||
| def model_dump(self, required_param: str) -> dict: | ||
| return {} | ||
|
|
||
| # Skips a class with a defined non-Pydantic `model_dump` method. | ||
| self.assertEqual(utils.clean({"test": Dummy()}), {}) | ||
| # previously python 2 code would cause an error while cleaning, | ||
| # and this entire object would be None, and we would log an error | ||
| # let's allow ourselves to clean `Dummy` as None, | ||
| # without blatting the `test` key | ||
| assert utils.clean({"test": Dummy()}) == {"test": None} |
There was a problem hiding this comment.
this test was hiding that the behaviour was determined by throwing an exception in an exception handler
and then swallowing that exception
| except Exception as exception: | ||
| item = ":".join(map(str, exception.args)) | ||
| log.warning("Error decoding: %s", item) | ||
| return None |
There was a problem hiding this comment.
not super convinced this logging behaviour is correct, but considering it was always throwing before now, it can't be too bad
2 participants
copyable setup block in readme so i don't have to figure it out again in future
we also had some python 3 unsafe code that we're luckily only calling in a safe way right now that mypy was incidentally warning us about. should do a patch release with this change