Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split word.py file into 4 files #8429

Closed
seblabbe opened this issue Mar 3, 2010 · 11 comments
Closed

Split word.py file into 4 files #8429

seblabbe opened this issue Mar 3, 2010 · 11 comments

Comments

@seblabbe
Copy link
Contributor

seblabbe commented Mar 3, 2010

The file word.py is getting very huge and forces one to create new classes inside of it (see below for explanation) which will get the file word.py even more huge in the future...

If a file contains the following :

#file1.py
class A:
    #huge class
    pass
class C(A):
    #huge class
    pass

one can not create a new class between A and C in another file (because of loops of import) :

#file1.py
class A:
    #huge class
    pass
from file2 import B
class C(B):
    #huge class
    pass
#file2.py
from file1 import A
class B(A)
    #large intermediate class
    pass

So the solution is either to put everything in the same file or to put everything in different files. In this case, I choose the last solution because word.py is getting huge.

This ticket removes Word_class, FiniteWord_class and InfiniteWord_class from word.py and put them in new files called respectively abstrac_word.py, finite_word.py and infinite_word.py.

CC: @sagetrac-abmasse

Component: combinatorics

Author: Sébastien Labbé

Reviewer: Alexandre Blondin Massé

Merged: sage-4.4.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/8429

@seblabbe
Copy link
Contributor Author

seblabbe commented Mar 3, 2010

comment:1

I will post a patch as soon as #8418 gets a positive review, because the patch will depend on it.

@seblabbe
Copy link
Contributor Author

seblabbe commented Mar 6, 2010

Attachment: trac_8429_split_word-sl.patch.gz

Depends on many tickets already merged in 4.3.4.alpha1

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

comment:3

I tested Sébastien's patch after having applied many tickets of the sage-combinat server. Here is the list

#7420 #7420 #7421 #7976 #7976 #7921 #7921 #7938 #8028 #8001 #8001 #5524 #8095 #8200 #8044 #8093 #8093 #8140 #8140 #8140 #6775 #8186 #8186 #8120 #7978 #8127 #8127 #8215 #8154 #8250 #8250 #8224 #8223 #8232 #8259 #8259 #8187 #8187 #8227 #8227 #8268 #8268 #8289 #8289 #8318 #8313 #7520 #7619 #8294 #8276 #8276 #8276 #8273 #8273 #8367 #8266 #8266 #8266 #8233 #8353 #8353 #8418 #8418 #8418 #8296 #7448 #8475 #8422 #8429

All tests passed, and the documentation buils correctly.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

comment:4

There seems to be three patches that have not been merged to sage-4.3.4 yet, but I don't think they are needed for #8429. Here is the list:

#8475, #8313, #8296.

I shouldn't have included #8429 as it is this current patch. I'll check for the three above patches if they are a problem.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

comment:5

Sorry, remove #8313 from the list, that was a typo. So the only tickets to look at are #8475 and #8296.

@seblabbe
Copy link
Contributor Author

seblabbe commented Mar 8, 2010

comment:6

Replying to @sagetrac-abmasse:

Sorry, remove #8313 from the list, that was a typo. So the only tickets to look at are #8475 and #8296.

Alexandre, I just moved the patch trac_8429_split_word-sl.patch before #8475 and #8296 in the sage-combinat tree. I can confirm that they commute so that this ticket doesn't "mercurialy" depend on #8296 and #8475.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

comment:7

I restested the patch after having applied all above listed patches except #8296 and #8475 and all tests passed ! The documentation builds fine too, so I give this patch a positive review. It is very unlikely that another patch could be in conflict with this one.

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

Reviewer: Alexandre Blondin Massé

@sagetrac-abmasse
Copy link
Mannequin

sagetrac-abmasse mannequin commented Mar 8, 2010

Author: Sébastien Labbé

@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha0

@jhpalmieri
Copy link
Member

comment:8

Merged "trac_8429_split_word-sl.patch" into 4.4.alpha0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants