Hi André, Thank you for starting off this discussion! This is a fun topic, so I’m keen on seeing what the rest of the folks in the PyIceberg community think as well :)
I’m of the opinion that ‘assert’ should only be used within test suites, because setting the optimize flag (-O) in the Python interpreter can disable asserts. And I agree with Piotr that having two separate flows available within Production code that can be triggered with the flag will make it more difficult for the community to debug specific behaviors. For this reason, the Ruff linter also checks for the usage of assert statements in S101: https://docs.astral.sh/ruff/rules/#flake8-bandit-s Sung On Sat, Oct 12, 2024 at 2:40 PM Piotr Findeisen <piotr.findei...@gmail.com> wrote: > Hi Andre, > > I am not very familiar with PyIceberg, but i am always for ensuring that > assumptions in our code are validated. > > I am not quite sure that assert is the way to go though. > In Java, one typically does not use `assert`, which can be enabled or > disabled. > checkState / checkArgument are preferred, because they are always on. > There are two important reasons: validating assumptions in test > (non-production) code is usually useless. The test outcome provide mostly > already the necessary coverage. It's much more useful to validate > assumptions always. > Having assert that are disabled or enabled means there are two flows of > the code (assert expression can have side effects!), how do you test for > that? Having one flow is a simplification. > This is a reasoning for Java codebase, but I don't think arguments are > language-specific, so I believe same can be argued about python code too. > > Best > Piotr > > > > > > On Thu, 10 Oct 2024 at 05:26, André Luis Anastácio > <ndrl...@proton.me.invalid> wrote: > >> Hello Everyone, >> >> I would like to open a discussion about using "assert" in some functions >> to promote a more defensive programming approach, ensuring that certain >> assumptions in our code are always validated. >> >> The intention here is to propose a recommendation, not a strict rule. >> What are your thoughts on this? >> >> In the Java implementation repository, we have some code that follows >> this approach in Scala code [1]. I'm not very familiar with Scala, so I’m >> not sure if this is a common pattern, but I believe we could improve the >> quality of our Python code by adopting a similar approach. >> >> You can find a reference discussing this approach here >> https://ratfactor.com/cards/tiger-style >> >> [1] >> https://github.com/search?q=repo%3Aapache%2Ficeberg+assert++language%3AScala&type=code&l=Scala >> >> Best regards, >> >> André Anastácio >> >
