#10025 closed defect (fixed)
sloane_sequence(111776)[2][:36] != sloane.A111776.list(36)
Reported by: | donmorrison | Owned by: | sage-combinat |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.6.1 |
Component: | combinatorics | Keywords: | combinat sloane |
Cc: | sage-combinat | Merged in: | sage-4.6.1.alpha0 |
Authors: | Yann Laigle-Chapuy, Nathann Cohen | Reviewers: | Nathann Cohen, Yann Laigle-Chapuy |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
sage: sloane_sequence(111776)[2][:36] != sloane.A111776.list(36)
Searching Sloane's online database...
True
sage: sloane_sequence(111776)[2][:36] == sloane.A111776.list(36)
Searching Sloane's online database...
False
sage: sloane_sequence(111776)[2][:36]
Searching Sloane's online database...
[1, 1, 1, 1, 2, 3, 1, 4, 6, 10, 1, 8, 12, 20, 35, 1, 16, 24, 40, 70, 125, 1, 32, 48, 80, 140, 250, 450, 1, 64, 96, 160, 280, 500, 900, 1625]
sage: sloane.A111776.list(36)
[1, 1, 1, 2, 1, 2, 3, 2, 1, 3, 4, 2, 3, 2, 4, 5, 1, 2, 4, 2, 5, 6, 4, 2, 3, 5, 4, 6, 7, 2, 5, 2, 1, 6, 4, 7]
sage: len(sloane_sequence(111776)[2][:36]) == len(sloane.A111776.list(36))
Searching Sloane's online database...
True
Attachments (2)
Change History (17)
comment:1 Changed 8 years ago by
- Report Upstream changed from Reported upstream. Little or no feedback. to N/A
Changed 8 years ago by
comment:2 Changed 8 years ago by
- Status changed from new to needs_review
The given patch does the following:
- rename A111776 to it's correct name A109814
- correct the discrepancy with the online version
- correct an offset problem with A000272 (there *is* one tree with no node)
- add an optional long doctest requiring internet to test tha sage keeps on par with OEIS
comment:3 Changed 8 years ago by
- Milestone changed from sage-4.6.1 to sage-4.6
comment:4 Changed 8 years ago by
- Cc sage-combinat added
comment:5 Changed 8 years ago by
- Milestone changed from sage-4.6 to sage-4.6.1
4.6 is feature freezed...
comment:6 Changed 8 years ago by
Hello !!!
Here is another patch to add to yours... The doctests did not pass, as the sloane_sequence method always outputs "Searching Sloane's online database...". I tried to find ways around, then thought the best was to just let it forward a verbose parameter to find_sloane... Anyway, I made this modification, and I also moved most of the documentation written in the init method of class A109814 just after the class definition. This way, it appears in the documentation when it is built.
The same should be done for the other methods, as currently very few of them appear : http://www.sagemath.org/doc/reference/sage/combinat/sloane_functions.html
Only my patch needs done to be reviewed before this ticket can be set as positively reviewed ! :-)
Nathann
Changed 8 years ago by
comment:7 follow-up: ↓ 8 Changed 8 years ago by
- Status changed from needs_review to needs_work
What version of Sage is your patch against? I get problems if I try to apply to sage-4.6.rc0.
drkirkby@hawk:~/sage-4.6.rc0/devel/sage-main$ hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/10025/trac_10025-smallfixes.patch adding trac_10025-smallfixes.patch to series file drkirkby@hawk:~/sage-4.6.rc0/devel/sage-main$ hg qpush applying trac_10025-smallfixes.patch patching file sage/combinat/sloane_functions.py Hunk #1 FAILED at 49 Hunk #2 FAILED at 8992 Hunk #3 FAILED at 9031 3 out of 3 hunks FAILED -- saving rejects to file sage/combinat/sloane_functions.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_10025-smallfixes.patch drkirkby@hawk:~/sage-4.6.rc0/devel/sage-main$
comment:8 in reply to: ↑ 7 Changed 8 years ago by
Replying to drkirkby:
What version of Sage is your patch against? I get problems if I try to apply to sage-4.6.rc0.
Well, I am working on 4.6.rc0 too... Though both patch need to be applied !
Nathann
comment:9 follow-up: ↓ 10 Changed 8 years ago by
Yes, i missed the first patch needed to be applied first.
Should this not return an error, rather than waste bandwidth looking on the database?
sage: sloane_find(32.23) Searching Sloane's online database... []
I's also puzzled that
sage: sloane_find(sin(x)) # where I have not defined x Searching Sloane's online database...
should actually produce any output, but it does.
I'm not really into OEIS, so perhaps I'm mistaken.
Dave
comment:10 in reply to: ↑ 9 Changed 8 years ago by
- Status changed from needs_work to needs_review
Replying to drkirkby:
I's also puzzled that
sage: sloane_find(sin(x)) # where I have not defined x Searching Sloane's online database...
Well, by default x is a symbolic variable, so "sin(x)" has some meaning already... Later on, I think the argument gets translated into a string, which still has meaning, and there shouldn't be anything wrong until the website answers that there is no sequence corresponding to this non-integer.
To be honest I had used this method once or twice only since I know Sage, and I already did not like the fact that it could not be told to stay mute (and also that verbosity is the default behaviour). Well, it's getting slowly fixed :-)
Nathann
comment:11 Changed 8 years ago by
Hi, Here, the primary goal is to correct the fact that sloane.A111776 is in fact the sequence A109814. I also added a test to check that be sure this doesn't happen again, which test every sequences against the online version. With respect to this, I'm happy with the changes made by Nathann and would give a positive review.
I do agree that it's a bad thing not to test the input for correctness, but it's not the point of this ticket. David, please feel free to open another one to adress this issue. I also let you make the decision about giving a positive review or not, as I'm a little bit biased as an author.
cheers,
Yann
comment:12 Changed 8 years ago by
I think it is better that you and Nathan review each others patches, as I don't consider myself qualified to check this - my pthon skills are not good. There is no problem in having two authors and two reviewers, as long as they review each others work, and not their own.
I guess I should open a ticket for the fact the code does not check its input, but there are more serious cases. Try
sage: seed(1,2)
for example.
Dave
comment:13 Changed 8 years ago by
- Reviewers set to Nathann Cohen, Yann Laigle-Chapuy
- Status changed from needs_review to positive_review
comment:14 Changed 8 years ago by
- Merged in set to sage-4.6.1.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 8 years ago by
Related note: sloane_find is now completely broken. See #10358.
A couple of points