Request for feedback on my first PR to SymPy

[Gabriela Stancu]

Hello everyone, 

I recently opened a PR regarding issue #6322 but it was closed with the concern that it was AI-generated with no feedback or explanation whatsoever. I would like to clarify that this is not the case and that the implementation was written by me. Since this is my first PR to SymPy, I was hoping to receive some technical feedback on the code and the approach as well.

If anyone from the community has the time, I would be very grateful if they could have a look at it and share a second opinion. I would also appreciate any feedback on the implementation itself and how I could improve it.

If needed, I am happy to open a new PR and discuss the approach again from the beginning.

Thank you very much for you time.

Best, 

Gabriela

[Krishna Chaitanya Bonu]

I had a look at it.

looks AI, you said your own code so here are some questions i have

1. why are there comments beside function definitions?

 result                    = dict(dict_1)

cancellation_flag = False

2. this doesnt look real

3. why are there import statements b/w the functions?? arent they supposed to be at the top?

4. why are there so many blank lines (blank lines b/w if and elif as well???)

profiles_stack    = []

5. why so many spaces before = sign ??

[Krishna Chaitanya Bonu]

Many things in the code changes just doesn't follow standard code practices like PEP-8.

please go through the sympy contribution guide. And try to follow all the standard conventions as much as possible.

[Gabriela Stancu]

Thank you for responding!

The spacing and alignment choices where my own stylistic choices to try to make the code look clearer to me. It is a formatting habit that I have been trying to follow for quite some time and I've seen it done by others as well. I was not aware that it conflicts with PEP-8 and SymPy's coding conventions. My focus when writing the patch was to first workout the method itself and make sure it behaved correctly. It was not my intension to cause confusion or tension, I was simply trying to present the code in a way that felt clear to me. Same applies to the import statement. I just added it near the code I was working on. If the implementation where to be considered further, I would of course change it to follow SymPy's code style. Should I rewrite it following PEP-8 and open a new pull request? 

[Krishna Chaitanya Bonu]

whether it will be considered further or not depends on maintainers and I am not one. and please go through the sympy contribution guide and follow everything before drafting a pr.

[Gabriela Stancu]

I appreciate the feedback. Thank you. 

--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/sympy/b49ef8a9-83dc-4940-ad0c-2f71f79d71d3n%40googlegroups.com.