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

Don't mention #940 as a problem anymore #26950

Closed
novoselt opened this issue Dec 24, 2018 · 11 comments
Closed

Don't mention #940 as a problem anymore #26950

novoselt opened this issue Dec 24, 2018 · 11 comments

Comments

@novoselt
Copy link
Member

It was gone, there is a test for it.

CC: @videlec @sagetrac-tmonteil

Component: interfaces

Author: Andrey Novoseltsev

Branch/Commit: 97eb56d

Reviewer: Frédéric Chapoton

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

@novoselt novoselt added this to the sage-8.6 milestone Dec 24, 2018
@novoselt
Copy link
Member Author

Branch: u/novoselt/drop940

@novoselt
Copy link
Member Author

Commit: 97eb56d

@novoselt
Copy link
Member Author

New commits:

97eb56dDo not mention the problem from #940 as it is gone

@videlec
Copy link
Contributor

videlec commented Dec 24, 2018

comment:3

Removing a comment is ok but removing a doctest is not.

@novoselt
Copy link
Member Author

comment:4

There is this test repeated by the patch from #940 later on in the file.

@fchapoton
Copy link
Contributor

comment:5

The other doctest is not exactly the same; it is missing the line

sage: a = octave.eval(t + ';') 

@novoselt
Copy link
Member Author

comment:6

And what exactly is this line testing? That eval works? Isn't it tested by eval itself? In the removed version it is claimed that it works fine, but presumably the other one was taking too much. My understanding is that the problem of #940 was gone and there is a test for it with the reference to that ticket. So I think there should not be a block in the module docstring that specifically says that there is a problem and #940 has to be fixed.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:7

ok, well. Then let it be..

@novoselt
Copy link
Member Author

comment:8

Thank you!

@vbraun
Copy link
Member

vbraun commented Dec 31, 2018

Changed branch from u/novoselt/drop940 to 97eb56d

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

4 participants