Hi,
I stumbled into misunderstanding the is_Set function from sets/set.py, 
thinking without looking that it determines the set property of a class. 
But it just does isinstance(X, Set_generic), so why not write that?

In https://trac.sagemath.org/ticket/24443 it emerges there are several such 
useless global functions and I propose to replace their usage with the code 
they contain. For example is_Ring(X) would be just X in Rings() so you can 
see that they are even used inconsistently (compare is_Set).

I'm told that devs need to agree for this refactoring so here it goes, 
please voice your opinion, you can see it's bad coding and prone to be 
misunderstood.

Now something new---if you agree with the above, I would like to experiment 
with a new procedure for refactoring tickets that aims to make reviewing 
them much easier, which will be needed for larger refactorings that touch a 
lot of files. My idea is to write script(s) that change the code, mostly 
using the rope library. The reviewer would then review the script, run it 
on a clean branch and compare if the changes match. If it can be made to 
work, inevitable merge conflicts could be dealt with by applying the 
scripts again (on the changed develop for example). I see automated 
refactoring as the only way to deal with large refactorings in Sage.

Please add your ideas or opinions on this, thanks in advance.

PS:
https://github.com/python-rope/rope
One problem is that rope cannot do Cython, so we're stuck with Python-only 
experiments at the moment.

PPS:
pycharm cannot be scripted I believe, and I cannot fix their bugs, it's 
closed source

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To post to this group, send email to sage-devel@googlegroups.com.
Visit this group at https://groups.google.com/group/sage-devel.
For more options, visit https://groups.google.com/d/optout.

Reply via email to