potiuk commented on PR #33786:
URL: https://github.com/apache/airflow/pull/33786#issuecomment-1714489727

   > @potiuk @eladkal The PR is still in the oven, yet I have a couple of 
questions:
   > 
   > 1. Can I assume that the name of a template argument (e.g., `foo`) is the 
same as its respective attribute (`self.foo`)? Otherwise, checking if it's 
assigned to the appropriate attribute becomes a very hard task (instead of 
simply recognizing pattern like `self.foo = foo`, which is easier to isolate 
from the code). I know it is quite strict, but I see no other choice.
   
   Fine for me. We are here for avoiding accidental mistakes not 100% accuracy, 
and it's fair to assume self.x = x is something that people use (I guess) in 
most cases. We can likely run some checks on existing codebase to turn it into 
actual datapoints (see below)
   
   > 2. To what depth should I try diving in when checking the usages of the 
template argument in the scope of this PR? Python's grammar is pretty 
[rich](https://docs.python.org/3/library/ast.html#abstract-grammar) - Does it 
sound reasonable to limit it to conditions or/and loops at this point?
   
   Wahtever helps with big percentage of use cases. and part of the 
implementation is to decide what is the low-effort/high coverage of cases. This 
is part of the difficulty - to make some assumptions that make sense. I don't 
think there is easy yes/no asnswer - but maybe running the check on existing 
codebase might give the answer on how often various patterns are used ? That 
could give a better datapoint that wild guessing I believe?
   
   > 3. Please take a look at the limitations that I mentioned in the PR's 
content - can we live with them for now?
   
   I think without knowing some datapoints - anyone can at most guess.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to