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

Fix the way sage/misc/sagedoc.py reads doc/common/builder.py #10351

Closed
jdemeyer opened this issue Nov 28, 2010 · 11 comments
Closed

Fix the way sage/misc/sagedoc.py reads doc/common/builder.py #10351

jdemeyer opened this issue Nov 28, 2010 · 11 comments

Comments

@jdemeyer
Copy link

The following code from sage/misc/sagedoc.py is horrible:

        # check if any documentation is missing.  first read the start
        # of SAGE_ROOT/devel/sage/doc/common/builder.py to find list
        # of languages, documents, and documents to omit
        doc_path = os.path.join(ROOT, 'devel', 'sage', 'doc')
        builder = open(os.path.join(doc_path, 'common', 'builder.py'))
        s = builder.read()[:1000]
        builder.close()
        # list of languages
        lang = []
        idx = s.find("LANGUAGES")
        if idx != -1:
            start = s[idx:].find('[')
            end =  s[idx:].find(']')
            if start != -1 and end != -1:
                lang = s[idx+start+1:idx+end].translate(None, "'\" ").split(",")
        # documents in SAGE_ROOT/devel/sage/doc/LANG/ to omit
        omit = []
        idx = s.find("OMIT")
        if idx != -1:
            start = s[idx:].find('[')
            end =  s[idx:].find(']')
            if start != -1 and end != -1:
                omit = s[idx+start+1:idx+end].translate(None, "'\" ").split(",")

Component: documentation

Author: Jeroen Demeyer

Reviewer: Mike Hansen, Volker Braun

Merged: sage-4.6.2.alpha4

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

@mwhansen
Copy link
Contributor

Reviewer: Mike Hansen

@mwhansen
Copy link
Contributor

comment:2

The method of saving the old path in this patch doesn't work since oldpath = sys.path has them both "pointing" to the same object. Then, when you change sys.path, you also change oldpath. You could change it to something like oldpath = sys.path[:]

@jdemeyer
Copy link
Author

comment:3

Replying to @mwhansen:

The method of saving the old path in this patch doesn't work since oldpath = sys.path has them both "pointing" to the same object. Then, when you change sys.path, you also change oldpath. You could change it to something like oldpath = sys.path[:]

Thanks! I've solved the problem with

sys.path = oldpath + [os.path.join(ROOT, 'devel', 'sage', 'doc', 'common')]

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 29, 2010

comment:4

I wonder if the docbuilder "options" should go into a separate file, such that we don't (have to) import the whole builder.py.

@jdemeyer
Copy link
Author

comment:5

Replying to @nexttime:

I wonder if the docbuilder "options" should go into a separate file, such that we don't (have to) import the whole builder.py.

That's a possibility, but I think the current approach works fine (and builder.py is not such a big file).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 29, 2010

comment:6

Replying to @jdemeyer:

Replying to @nexttime:

I wonder if the docbuilder "options" should go into a separate file, such that we don't (have to) import the whole builder.py.

That's a possibility, but I think the current approach works fine (and builder.py is not such a big file).

Well, importing the whole file could have very strange side-effects, at least in the future.

Another reason is users are likely to edit (just) these options. (Which however is sub-optimal by itself...)

@jdemeyer
Copy link
Author

comment:7

I can see your point, maybe it would not be a bad idea to split the configuration from the code...

@jdemeyer
Copy link
Author

Attachment: 10351_sagedoc.patch.gz

SAGELIB patch

@vbraun
Copy link
Member

vbraun commented Feb 1, 2011

Changed reviewer from Mike Hansen to Mike Hansen, Volker Braun

@vbraun
Copy link
Member

vbraun commented Feb 1, 2011

comment:10

Definite improvement ;-)

@jdemeyer
Copy link
Author

jdemeyer commented Feb 7, 2011

Merged: sage-4.6.2.alpha4

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

3 participants