input on ticket#32923

hello everyone, I am currently working on #32923 (Add Field._clean_bound_field() to remove isinstance check in BaseForm._clean_fields()) – Django
If I correctly grasp the situation, it appears that a solution has been proposed within the ticket. Could someone kindly provide guidance on the implemented changes? However, please feel free to correct me if my understanding is inaccurate.
Thank you!

I’m not sure what advice you are after but I can try!
Yes, Nick has written some code and has some questions. You should take that as a starting point and address those questions/thoughts.

This is a clean up/optimization ticket. This means you can check if the area being changed is already covered by tests, and if so, no further tests would be required.
There is a benchmarking project that could check if this improves performance :thinking:
An optimization shouldn’t require docs updates or a release note.

So give it a go! Once you’re happy, raise a PR and it will get feedback :+1:

One last thing, in general I avoid optimization tickets and wouldn’t recommend them as a first ticket.

You have to justify that this is better than what is there already, as all changes adds some churn and some risk. The way you prove this is complicated. It’s either “this is much simpler to understand” or showing a postive impact on performance. Not simple things.

Bug fixes and new features will have tests attached which prove the benefit of the changes. To me this is “easier” as it’s well defined.

1 Like

thanks @sarahboyce for the advice
PR - Fixed #32923 -- Refactored out Field._clean_bound_field(). by Waheedsys · Pull Request #17747 · django/django · GitHub

1 Like