Opened 7 years ago
Closed 7 years ago
#17818 closed enhancement (fixed)
Interface to FindStat
Reported by:  mantepse  Owned by:  

Priority:  major  Milestone:  sage6.7 
Component:  interfaces: optional  Keywords:  FindStat 
Cc:  vdelecroix, ncohen, stumpc5, VivianePons  Merged in:  
Authors:  Martin Rubey  Reviewers:  Christian Stump, Viviane Pons 
Report Upstream:  N/A  Work issues:  
Branch:  ec3bc2a (Commits, GitHub, GitLab)  Commit:  ec3bc2abce27759168e9d6ca5d3cb4a38a7dde4b 
Dependencies:  Stopgaps: 
Description (last modified by )
Sage should have an interface to http://www.findstat.org, the "combinatorial statistics finder". A first version, modelled after the interface to the OEIS is attached. To get you started, you can try the following:
sage: findstat('St000045') # optional  internet St000045: The number of linear extensions of the tree. sage: s = findstat(3); s # optional  internet St000003: The number of [[/StandardTableauxstandard Young tableaux]] of the partition. sage: s.browse() sage: stat = {pi: pi.length() for pi in Permutations(3)} sage: search = findstat(stat) ; search # optional  internet 0: (St000018: The [[/Permutations/Inversionsnumber of inversions]] of a permutation., [], 6) 1: (St000004: The [[/Permutations/DescentsMajormajor index]] of a permutation., ['inversionnumber to majorindex bijection'], 6) 2: (St000067: The inversion number of the alternating sign matrix., ['to alternating sign matrix'], 6) 3: (St000224: The sorting index of a permutation., ['first fundamental transformation'], 6) 4: (St000008: The major index of the composition., ['inversionnumber to majorindex bijection', 'descent composition'], 6) 5: (St000059: The inversion number of a standard Young tableau as defined by Haglund and Stevens. [1] , ['inversionnumber to majorindex bijection', 'RobinsonSchensted recording tableau'], 6) 6: (St000153: The number of adjacent cycles of a permutation., ['reverse', 'first fundamental transformation'], 6) 7: (St000156: The Denert index of a permutation., ['inverse first fundamental transformation', 'foata_bijection'], 6) 8: (St000174: The flush statistic on semistandard tableaux., ['to alternating sign matrix', 'to semistandard tableau'], 6)
Change History (121)
comment:1 Changed 7 years ago by
 Description modified (diff)
comment:2 Changed 7 years ago by
 Cc vdelecroix added
comment:3 followup: ↓ 5 Changed 7 years ago by
comment:4 Changed 7 years ago by
 Cc ncohen added
comment:5 in reply to: ↑ 3 Changed 7 years ago by
You mean, the text between examples is missing?
comment:6 Changed 7 years ago by
 Cc stumpc5 added
comment:7 Changed 7 years ago by
No, I mean for example
We can access references provided as follows: sage: s.references()
should be
We can access references provided as follows:: sage: s.references()
comment:8 Changed 7 years ago by
 Commit changed from ce07ce4d0cb545a054f6b4878ce349119d18d901 to d21fad39a7d500d98bacc42f901942a3caa46462
Branch pushed to git repo; I updated commit sha1. New commits:
d21fad3  fix formatting for EXAMPLES and cache the right method

comment:9 followup: ↓ 14 Changed 7 years ago by
Your new commit has the same problem. When you use "::" somewhere, the next line must be indented. To make sure that everything works as it should:
1) Add your new file to the documentation, i.e. the SAGE_ROOT/src/doc/en/reference/databases/index.rst file
2) compile the documentation sage b && sage docbuild reference/databases html
And, of course, read the html page corresponding to your new file to check that it is alright.
See also: http://www.sagemath.org/doc/developer/sage_manuals.html#addinganewfile
Nathann
comment:10 followup: ↓ 11 Changed 7 years ago by
Hello,
I really do understand neither your strategy nor your goal with this ticket.
oeis is a Sage completely independent project and we have this ugly parser because we do not have any choice: there is no API to access their database.
Findstat project highly uses Sage and you are one of the author. Instead of integrating findstat code inside Sage you decided to make a closed source project. Right, this is fine with GPL and this is your choice. Now you decide to interface your findstat project with Sage. As a consequence you oblige the user to have an internet access to play with findstat features. Fine, this is again your choice. Instead of providing a clean API from the findstat side (let say json answers from http queries) you chose to implement an ugly HTML parser with regexp to get the information on the Sage side. This is inefficient and counter productive and you should seriously revise your strategy.
Vincent
comment:11 in reply to: ↑ 10 ; followup: ↓ 12 Changed 7 years ago by
I agree that the situation could be better from the sage point of view. However, there are a few things to keep in mind:
The current FindStat project really does want to be open to nonSageMath users, and it tries very hard. In fact, I believe that most of the effort went into the web interface. I think this is very important, so I make it boldface :)
Currently, I (Martin) am mainly interested in using FindStat from within SageMath. I asked Christian whether this would be possible, and so he provided me with the necessary information to create this interface.
I agree that a clean API is desirable, but it is beyond my knowledge, and beyond the human resources of the FindStat project. (As far as I know, there are currently five people who contributed to the FindStat project  all listed here: http://www.findstat.org/Contributors. My first and so far only contribution (apart from editing statistics) is the interface, but actually, this is more a contribution to SageMath, isn't it?)
Therefore, if you are able to provide such a clean interface, please go for it!
comment:12 in reply to: ↑ 11 ; followup: ↓ 13 Changed 7 years ago by
Replying to mantepse:
I agree that the situation could be better from the sage point of view. However, there are a few things to keep in mind:
The current FindStat? project really does want to be open to nonSageMath users, and it tries very hard. In fact, I believe that most of the effort went into the web interface. I think this is very important, so I make it boldface :)
Agreed. The website is nice as a website.
Currently, I (Martin) am mainly interested in using FindStat? from within SageMath. I asked Christian whether this would be possible, and so he provided me with the necessary information to create this interface.
This is where I do not agree. The solution to make it Sagemathfriendly is not the interface proposed in this ticket. There should be another web interface for interaction with Sage. This is not the hard part of the work.
I agree that a clean API is desirable, but it is beyond my knowledge, and beyond the human resources of the FindStat project. (As far as I know, there are currently five people who contributed to the FindStat project  all listed here: http://www.findstat.org/Contributors. My first and so far only contribution (apart from editing statistics) is the interface, but actually, this is more a contribution to SageMath, isn't it?)
In its current version, I am not sure this interface does more good to Sagemath than to Findstat.
Therefore, if you are able to provide such a clean interface, please go for it!
I would if the code was freely available... please go release it!
Vincent
comment:13 in reply to: ↑ 12 Changed 7 years ago by
This is where I do not agree. The solution to make it Sagemathfriendly is not the interface proposed in this ticket. There should be another web interface for interaction with Sage. This is not the hard part of the work.
It would be for me. Besides, in which way precisely would this be better for SageMath?
In its current version, I am not sure this interface does more good to Sagemath than to Findstat.
I admit, this is not really important for me, I shouldn't have raised this point. I am mainly a mathematician. If this interface is useful to you, use it, otherwise don't. I was asked to polish the code so it is usable for people other than me, so I spent a few days on polishing. If it doesn't go into SageMath that's fine with me. I hope I will still be able to use it myself.
Therefore, if you are able to provide such a clean interface, please go for it!
I would if the code was freely available... please go release it!
I do not understand why you think that I am in the position to do that. FindStat? is not my website, not my code.
comment:14 in reply to: ↑ 9 ; followup: ↓ 15 Changed 7 years ago by
Replying to ncohen:
Your new commit has the same problem.
2) compile the documentation
sage b && sage docbuild reference/databases html
Hi Nathann!
Many thanks for your help, but I'm afraid I need more. Is there a way to make docbuild tell me where I made a mistake? I obtain
[databases] loading pickled environment... not yet created [databases] building [html]: targets for 10 source files that are out of date [databases] updating environment: 10 added, 0 changed, 0 removed [databases] Encoding error: [databases] 'ascii' codec can't decode byte 0xe2 in position 24871: ordinal not in range(128) [databases] The full traceback has been saved in /tmp/sphinxerrYMbReV.log, if you want to report the issue to the developers. Build finished. The built documents can be found in /home/martin/sage/src/doc/output/html/en/reference/databases
but I have no idea what that traceback is trying to tell me:
Traceback (most recent call last): File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/cmdline.py", line 254, in main app.build(force_all, filenames) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/application.py", line 212, in build self.builder.build_update() File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/builders/__init__.py", line 214, in build_update 'out of date' % len(to_build)) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/builders/__init__.py", line 234, in build purple, length): File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/builders/__init__.py", line 134, in status_iterator for item in iterable: File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/environment.py", line 480, in update_generator self.read_doc(docname, app=app) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/environment.py", line 627, in read_doc pub.publish() File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/core.py", line 217, in publish self.settings) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/readers/__init__.py", line 72, in read self.parse() File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/readers/__init__.py", line 78, in parse self.parser.parse(self.input, document) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/__init__.py", line 172, in parse self.statemachine.run(inputlines, document, inliner=self.inliner) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 170, in run input_source=document['source']) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 239, in run context, state, transitions) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 460, in check_line return method(match, context, next_state) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2726, in underline self.section(title, source, style, lineno  1, messages) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 327, in section self.new_subsection(title, lineno, messages) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 395, in new_subsection node=section_node, match_titles=True) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 282, in nested_parse node=node, match_titles=match_titles) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 195, in run results = StateMachineWS.run(self, input_lines, input_offset) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 239, in run context, state, transitions) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 460, in check_line return method(match, context, next_state) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2301, in explicit_markup self.explicit_list(blank_finish) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2331, in explicit_list match_titles=self.state_machine.match_titles) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 319, in nested_list_parse node=node, match_titles=match_titles) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 195, in run results = StateMachineWS.run(self, input_lines, input_offset) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 239, in run context, state, transitions) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/statemachine.py", line 460, in check_line return method(match, context, next_state) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2604, in explicit_markup nodelist, blank_finish = self.explicit_construct(match) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2311, in explicit_construct return method(self, expmatch) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2054, in directive directive_class, match, type_name, option_presets) File "/home/martin/sage/local/lib/python2.7/sitepackages/docutils/parsers/rst/states.py", line 2103, in run_directive result = directive_instance.run() File "/home/martin/sage/src/doc/common/sage_autodoc.py", line 1166, in run documenter.generate(more_content=self.content) File "/home/martin/sage/src/doc/common/sage_autodoc.py", line 638, in generate self.analyzer = ModuleAnalyzer.for_module(self.real_modname) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/pycode/__init__.py", line 204, in for_module obj = cls.for_file(source, modname) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/pycode/__init__.py", line 187, in for_file obj = cls(fileobj, modname, filename) File "/home/martin/sage/local/lib/python2.7/sitepackages/Sphinx1.2.2py2.7.egg/sphinx/pycode/__init__.py", line 224, in __init__ self.code = self.source.read().decode(self.encoding) UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 24871: ordinal not in range(128)
comment:15 in reply to: ↑ 14 ; followup: ↓ 16 Changed 7 years ago by
Hello,
Many thanks for your help, but I'm afraid I need more. Is there a way to make docbuild tell me where I made a mistake?
[databases] Encoding error: [databases] 'ascii' codec can't decode byte 0xe2 in position 24871: ordinal not in range(128) [databases] The full traceback has been saved in /tmp/sphinxerrYMbReV.log, if you want to report the issue to the developers.
Well, Sphinx says that there is a nonascii character at position 24871 in some file. Have you checked whether there might be a nonascii character at position 24871 in your file ?
Nathann
comment:16 in reply to: ↑ 15 ; followup: ↓ 17 Changed 7 years ago by
Replying to ncohen:
Well, Sphinx says that there is a nonascii character at position 24871 in some file. Have you checked whether there might be a nonascii character at position 24871 in your file ?
Now I found it! It is a long dash in a reference retrieved from FindStat?. I have to redo the doctests anyway, because the descriptions, names, references, etc., are all subject to change...
Thank you!
comment:17 in reply to: ↑ 16 Changed 7 years ago by
Now I found it! It is a long dash in a reference retrieved from FindStat?. I have to redo the doctests anyway, because the descriptions, names, references, etc., are all subject to change...
There must be something wrong with dashes. Every single time I get this error message it is because of a dash between the pages in the reference of a paper, i.e. 234190.
Let's call this message "the dash message" from now on.
Nathann
comment:18 Changed 7 years ago by
 Cc VivianePons added
comment:19 Changed 7 years ago by
To answer a bit what has been said here. I do agree that it would be better to provide a proper API on the FindStat? side, that was my idea at first. But, it's not that easy, we're not that many, and nobody did it! On the other hand, Martin wanted the Sage interface because he needed it. And the dirty html parsing was the quickest way to get it. Even if it's not perfect, I still think it could be useful for other people, that's why he created the ticket.
If you want more of a technical reason: at this stage, FindStat? is no more than a customized MoinMoin? macro. Especially, we don't really deal with the http request: we just take some info from MoinMoin? and give some html back to be included in a MoinMoin? html page, we have no interface to deal with non html responses.
For similar reasons, I don't see how to include the FindStat? search directly into Sage at the moment, it demands lots of work. I'm not saying it's never going to happen, but probably not just right now.
About opensourceness, the main reasons it's not been open yet are mostly time and technical issues. FindStat? is not a software, it's a webservice. In the meantime, we've welcomed anyone wanting to work on the project!
comment:20 Changed 7 years ago by
 Commit changed from d21fad39a7d500d98bacc42f901942a3caa46462 to d0ef23a9f915bc6be84f0cf4b71a2728708be2ff
comment:21 Changed 7 years ago by
 Commit changed from d0ef23a9f915bc6be84f0cf4b71a2728708be2ff to 6caecea9f006c0b6637025685d9ebc5b992dbc5e
Branch pushed to git repo; I updated commit sha1. New commits:
6caecea  fix oversight

comment:22 Changed 7 years ago by
I tried (with a lot of help from the FindStat? team) to answer the (technical) objections raised above. In case somebody is interested in having this finalized, I need a little help with the following items:
1) documentation and doctests
2) I believe that statistics, collections and maps should have something similar to a unique representation, but I'm not sure how to achieve this, especially for statistics. I should mention that it is quite possible that at some point collections and maps can be edited, too. So the idea is to have caching enabled for objects with id different from 0...
3) Christian told me that I need to do something like lazy import, but I currently don't really know what that is. I simply followed oeis.py
...
4) a design question: it appears that, in sage, python properties are hardly used at all. I have no idea why. Currently, the interface uses properties to allow editing statistics (collections and maps currently cannot be edited). This implies some inconsistencies. Therefore:
 should I change readonly properties like
FindStatStatistic.id
to ordinary methods?  should I do the same with the other properties, i.e., provide methods
FindStatStatistic.description
andFindStatStatistic.set_description
?
comment:23 Changed 7 years ago by
 Commit changed from 6caecea9f006c0b6637025685d9ebc5b992dbc5e to 9adb2b5db3f77f1a0fcadd665bcc5cf6ee73cf77
Branch pushed to git repo; I updated commit sha1. New commits:
9adb2b5  make collections and maps hashable, fix bug for Cores, fix bug in equality for collections

comment:24 Changed 7 years ago by
Hello,
Looks better now ;)
Could you use a lazy import in the "all.py". What it means is that you have to replace
from sage.databases.findstat import findstat
with
from sage.misc.lazy_import import lazy_import lazy_import('sage.databases.findstat', 'findstat')
What it changes is that you will have:
sage: type(findstat) <type 'sage.misc.lazy_import.LazyImport'>
but mostly nothing else (access to method, tab completion, documentation is identical). The advantage is that it is not imported on startup. It would be really good since you are doing a lot of global imports in your module.
Vincent
comment:25 Changed 7 years ago by
 Commit changed from 9adb2b5db3f77f1a0fcadd665bcc5cf6ee73cf77 to 8817ab3025c69927894e5928a5c9f7aa5062e879
Branch pushed to git repo; I updated commit sha1. New commits:
8817ab3  lazy import findstat

comment:26 Changed 7 years ago by
 Commit changed from 8817ab3025c69927894e5928a5c9f7aa5062e879 to 05a08edd8a0f0a8532a1fba6fead85e4d97e42de
Branch pushed to git repo; I updated commit sha1. New commits:
05a08ed  more doctesting, fix bug in FindStatCollection.__init__

comment:27 Changed 7 years ago by
Plenty of documentation issues! Did you try to run make doc
?
 After
INPUT:
,OUTPUT:
there must be a blankline.
TODO
sections must be indented, i.e... TODO:: we definitely should do this and that
 The first line of the documentation should be an imperative and start with an uppercase. Replace
returns the name of the collection
withReturn the name of the collection
And no point at the end of this first sentence.
 This is wrong and will raise an error during the creation of the documentation.
EXAMPLES:: A particular statistic can be called by its Stnumber or number::
There should be two colon only if the next block is indented. In that case it should beEXAMPLES: A particular statistic can be called by its Stnumber or number::
And in the following you forgot a colonTo search for a distribution, send a list of lists, or a single pair:
There is a lot of useful information in the developer guide.
comment:28 Changed 7 years ago by
 Commit changed from 05a08edd8a0f0a8532a1fba6fead85e4d97e42de to fefc6c2a8e466e5afaa608be6f3ffe540b21e26f
Branch pushed to git repo; I updated commit sha1. New commits:
fefc6c2  improving documentation: make doc passes

comment:29 Changed 7 years ago by
 Commit changed from fefc6c2a8e466e5afaa608be6f3ffe540b21e26f to 14def60afb6300516bda1b4b0d1d8c277a154f4c
Branch pushed to git repo; I updated commit sha1. New commits:
14def60  better formatting and more documentation

comment:30 Changed 7 years ago by
 Commit changed from 14def60afb6300516bda1b4b0d1d8c277a154f4c to 52330b9e17e7c58aa335900af13cfb4ccaccea83
Branch pushed to git repo; I updated commit sha1. New commits:
52330b9  unicode tempfile

comment:31 Changed 7 years ago by
 Commit changed from 52330b9e17e7c58aa335900af13cfb4ccaccea83 to 61d0c69546ac4ca4c8151ab319aabff17e94bc42
Branch pushed to git repo; I updated commit sha1. New commits:
7c407bd  Merge git://trac.sagemath.org/sage into public/findstatinterface

fd6b6c3  Merge branch 'public/findstatinterface' of git://trac.sagemath.org/sage into public/findstatinterface

4c48615  Merge branch 'public/findstatinterface' of git://trac.sagemath.org/sage into public/findstatinterface

61d0c69  properly htmlescape

comment:32 Changed 7 years ago by
 Commit changed from 61d0c69546ac4ca4c8151ab319aabff17e94bc42 to 0702e72f4f2626dd5fee2e8db6d1311f2215ea6e
Branch pushed to git repo; I updated commit sha1. New commits:
0702e72  replace properties by setters, improve documentation

comment:33 Changed 7 years ago by
 Commit changed from 0702e72f4f2626dd5fee2e8db6d1311f2215ea6e to 4ba6ef012b4581b8dc5bd277c556c8290b84d441
Branch pushed to git repo; I updated commit sha1. New commits:
4ba6ef0  more documentation, bugfix in in_range, findstat accepts FindStatCollection

comment:34 Changed 7 years ago by
 Status changed from new to needs_review
comment:35 Changed 7 years ago by
 Status changed from needs_review to needs_work
Hi Martin, thanks for all your work on that!
 Several doctests currently fail:
sage t src/sage/databases/findstat.py ********************************************************************** File "src/sage/databases/findstat.py", line 637, in sage.databases.findstat.FindStatStatistic.id Failed example: findstat(1).id() # optional  internet Expected: <BLANKLINE> (process:...): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed 1 Got: 1 ********************************************************************** File "src/sage/databases/findstat.py", line 719, in sage.databases.findstat.FindStatStatistic.function Failed example: findstat(lambda pi: pi.length(), "Permutations").function() # optional  internet Expected: <function <lambda> at ...> Got: <BLANKLINE> (process:21380): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed <function <lambda> at 0x7fb420e806e0> ********************************************************************** File "src/sage/databases/findstat.py", line 1294, in sage.databases.findstat.FindStatCollection.first_terms Failed example: c.first_terms(lambda x: 1, max_values=10)) # optional  internet Exception raised: Traceback (most recent call last): File "/home/stumpc5/progs/sagegit/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 488, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/stumpc5/progs/sagegit/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 848, in compile_and_execute compiled = compiler(example) File "/home/stumpc5/progs/sagegit/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 486, in <lambda> example.source, filename, "single", compileflags, 1) File "<doctest sage.databases.findstat.FindStatCollection.first_terms[2]>", line 1 c.first_terms(lambda x: Integer(1), max_values=Integer(10))) # optional  internet ^ SyntaxError: invalid syntax ********************************************************************** File "src/sage/databases/findstat.py", line 1505, in sage.databases.findstat.FindStatMap.id Failed example: m = findstat(lambda pi: pi.length(), "Permutations")[1][1][0] # optional  internet Expected nothing Got: <BLANKLINE> (process:21392): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed <BLANKLINE> (process:21402): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed ********************************************************************** 4 items had failures: 1 of 4 in sage.databases.findstat.FindStatCollection.first_terms 1 of 3 in sage.databases.findstat.FindStatMap.id 1 of 2 in sage.databases.findstat.FindStatStatistic.function 1 of 2 in sage.databases.findstat.FindStatStatistic.id [77 tests, 4 failures, 12.80 s]  sage t src/sage/databases/findstat.py # 4 doctests failed  Total time for all tests: 13.1 seconds cpu time: 5.7 seconds cumulative wall time: 12.8 seconds
 The documentation doesn't build (unless I made a mistake, I haven't been building docs in a while):
Error building the documentation. Note: incremental documentation builds sometimes cause spurious error messages. To be certain that these are real errors, run "make docclean" first and try again. Traceback (most recent call last): File "/home/stumpc5/progs/sagegit/src/doc/common/builder.py", line 1618, in <module> getattr(get_builder(name), type)() File "/home/stumpc5/progs/sagegit/src/doc/common/builder.py", line 503, in _wrapper x.get(99999) File "/home/stumpc5/progs/sagegit/local/lib/python/multiprocessing/pool.py", line 558, in get raise self._value OSError: [databases] /home/stumpc5/progs/sagegit/local/lib/python2.7/sitepackages/sage/databases/findstat.py:docstring of sage.databases.findstat:16: ERROR: Unexpected indentation.
 I wonder whether it is okay that doctests open browser windows, @vdelecroix, @ncohen.
 How should doctests that change from the FindStat? side over time be handled, @vdelecroix, @ncohen. One (possibly not the best) example is
sage: [FindStatCollection(c.first_terms(lambda x: 0, max_values=1)[0][0]) for c in cl] [Cc0001: Permutations, Cc0002: Integer partitions, Cc0005: Dyck paths, Cc0006: Integer compositions, Cc0007: Standard tableaux, Cc0009: Set partitions, Cc0010: Binary trees, Cc0012: Perfect matchings, Cc0013: Cores, Cc0014: Posets, Cc0017: Alternating sign matrices, Cc0018: GelfandTsetlin patterns, Cc0019: Semistandard tableaux, Cc0020: Graphs, Cc0021: Ordered trees, Cc0022: Finite Cartan types, Cc0023: Parking functions]
comment:36 Changed 7 years ago by
I was trying to fix the doctests, but stumble across the following problem: I constantly get errors such as
Failed example: FindStatCollection("Binary trees").name() # optional  internet Expected: u'Binary tree' Got: <BLANKLINE> (process:23292): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed u'Binary tree'
and the problem is that this
<BLANKLINE> (process:23292): GLibCRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed
only happens sometimes (google search tells me this appears to be a firefox issue, but I wouldn't know how to handle this in Sage doctests.
comment:37 Changed 7 years ago by
 Commit changed from 4ba6ef012b4581b8dc5bd277c556c8290b84d441 to 598ac1e8ceaacc000cc38bf1818e1636a5b63d15
Branch pushed to git repo; I updated commit sha1. New commits:
598ac1e   Removed the colons of the end of the first doc lines.

comment:38 Changed 7 years ago by
 Commit changed from 598ac1e8ceaacc000cc38bf1818e1636a5b63d15 to 5b39d53191356f852771b256fd1d609c2cc0429e
Branch pushed to git repo; I updated commit sha1. New commits:
5b39d53  fixed typo in the documentation that make it fail to build

comment:39 Changed 7 years ago by
Christian, the last commit is not following the conventions at http://www.sagemath.org/doc/developer/coding_basics.html#thedocstringofafunctioncontent:
It [the one sentence description] must be followed by a blank line and end in a period.
Please revert.
New commits:
5b39d53  fixed typo in the documentation that make it fail to build

comment:40 Changed 7 years ago by
I'm still having the problem that I cannot run the doctests at all. As I mentioned on sagedevel, I get, for example:
martin@MartinLaptop:~/sage$ ./sage t optional=internet src/sage/databases/findstat.py too few successful tests, not using stored timings Running doctests with ID 20150330104121fba377bd. Git branch: public/findstatinterface Doctesting 1 file. sage t src/sage/databases/findstat.py ********************************************************************** File "src/sage/databases/findstat.py", line 1152, in sage.databases.findstat.FindStatCollection.__init__ Failed example: FindStatCollection("Dyck paths") # optional  internet Exception raised: Traceback (most recent call last): File "/home/martin/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 488, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/martin/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 850, in compile_and_execute exec(compiled, globs) File "<doctest sage.databases.findstat.FindStatCollection.__init__[0]>", line 1, in <module> FindStatCollection("Dyck paths") # optional  internet NameError: name 'FindStatCollection' is not defined
How can I fix this? FindStatCollection?
is imported in the preceding line of the docstring!
comment:41 Changed 7 years ago by
Is I wrote some time earlier, you must run
sage t optional=sage,internet path/to/interface.sage
If you only run optional=internet
, you are *only* running those tests having # optional  internet
, so all the imports (and a few others) are not performed.
comment:42 Changed 7 years ago by
Even for a ".py" file? OK, yes, it works. Thanks!
Regarding your comments above:
3.) I agree. Removing these doctests also gets rid of the annoying firefox bug.
4.) This test is there precisely because *if* it doesn't pass, the interface *should* be updated (in the case at hand, with the information concerning new combinatorial collections.
Could you please revert your commit with the periods, then I fix the other stuff! Thanks also for fixing the docbuild!
comment:43 followup: ↓ 46 Changed 7 years ago by
4.) This test is there precisely because *if* it doesn't pass, the interface *should* be updated (in the case at hand, with the information concerning new combinatorial collections.
My point here is: doctests can stop working without any change on the Sage side. This implies that a new Sage release possibly depends on someone fixing the doctests in the FindStat interface. I am not sure this is the right thing to do...
comment:44 Changed 7 years ago by
(the doctests were working, but I now get back the unexpected indention
, that's why I haven't yet pushed.
comment:45 Changed 7 years ago by
 Commit changed from 5b39d53191356f852771b256fd1d609c2cc0429e to d604a77e06fda46c4f560ee267a033eda1672855
Branch pushed to git repo; I updated commit sha1. New commits:
d604a77   Added back the periods at the end of the first line of the docs.

comment:46 in reply to: ↑ 43 ; followup: ↓ 48 Changed 7 years ago by
Replying to stumpc5:
4.) This test is there precisely because *if* it doesn't pass, the interface *should* be updated (in the case at hand, with the information concerning new combinatorial collections.
My point here is: doctests can stop working without any change on the Sage side. This implies that a new Sage release possibly depends on someone fixing the doctests in the FindStat interface. I am not sure this is the right thing to do...
Well, but if this doctest (as currently written) doesn't pass, the interface will not work reliably: user might get a result where her statistic is produced via a map from a statistic on the new collection.
The constant _findstat_collections
is really the only place that needs updating, and it should be easy to update it. But it does need updating...
New commits:
d604a77   Added back the periods at the end of the first line of the docs.

comment:47 Changed 7 years ago by
Okay, found the problem. I added your examples from the class FindStat? also to the beginning of the file in order to make the reference manual nicer to read. Feel free to update that again if you prefer another way of doing it.
comment:48 in reply to: ↑ 46 Changed 7 years ago by
Replying to mantepse:
Well, but if this doctest (as currently written) doesn't pass, the interface will not work reliably: user might get a result where her statistic is produced via a map from a statistic on the new collection.
I am not necessarily arguing about doctest that tests whether the interface still works well (even though my general concern also applies to those, and e.g. the OEIS interface was broken several times IIRC).
What about the doctest
sage: print findstat(1).description() # optional  internet The number of ways to write a permutation as a minimal length product of simple transpositions.
after someone has changed the description of this statistic? Or the case that we added another collection  would that break the interface, or would it only not serve the new collection as long as it is not added?
comment:49 Changed 7 years ago by
Yes, I agree that there is a problem with the other doctests, involving name, description, code, perhaps even values... But I don't really know what to do about this. I think it would be a shame if the interface cannot go in just because of this.
A workaround might be to mark these EXAMPLES as random. Once full search is implemented, I could have doctests that search for the description. But I won't do this now.
The only doctest which is really necessary is the test in the FindStatCollection? class.
comment:50 Changed 7 years ago by
Concerning the introductory section: I'm going to think of a systematic tour, which demonstrates one or two typical workflows. Thanks for catching the error!
Concerning the webbrowser: the simple and correct solution is to remove the keyword "internet" from the webbrowser tests!
comment:51 Changed 7 years ago by
 Commit changed from d604a77e06fda46c4f560ee267a033eda1672855 to b97b209f902f1ba0bcf89d459b89d43c605ed0b9
comment:52 Changed 7 years ago by
 Commit changed from b97b209f902f1ba0bcf89d459b89d43c605ed0b9 to 6752f09313131d7fe4fc0b91cb4dbd10a3e8dece
Branch pushed to git repo; I updated commit sha1. New commits:
6752f09  make doctest random whenever necessary

comment:53 Changed 7 years ago by
 Commit changed from 6752f09313131d7fe4fc0b91cb4dbd10a3e8dece to 8272b8337c1cdf230fbb53c291684b2a79aebae8
Branch pushed to git repo; I updated commit sha1. New commits:
8272b83  cosmetics for doctests

comment:54 Changed 7 years ago by
 Commit changed from 8272b8337c1cdf230fbb53c291684b2a79aebae8 to 51ff4ef005cbca824a9b471ce310b4669f2f9e02
Branch pushed to git repo; I updated commit sha1. New commits:
51ff4ef  make code of maps available, add todo for introduction

comment:55 Changed 7 years ago by
 Commit changed from 51ff4ef005cbca824a9b471ce310b4669f2f9e02 to cd07f7769907d815498b30471804a55d637f8cfc
Branch pushed to git repo; I updated commit sha1. New commits:
cd07f77  unified the FINDSTAT_URL parameters

comment:56 Changed 7 years ago by
 Commit changed from cd07f7769907d815498b30471804a55d637f8cfc to 01e7ca429c1064cef0c0ffca643b9826456c8f8f
Branch pushed to git repo; I updated commit sha1. New commits:
01e7ca4  include guided tour, expose name of code of maps, fixes to references, equality and inequality

comment:57 Changed 7 years ago by
 Status changed from needs_work to needs_review
comment:58 Changed 7 years ago by
 Commit changed from 01e7ca429c1064cef0c0ffca643b9826456c8f8f to 5212c0184e61ebd162ca856d8f61f8195f2405b8
Branch pushed to git repo; I updated commit sha1. New commits:
5212c01  fix missing parenthesis. I hate python.

comment:59 Changed 7 years ago by
 Commit changed from 5212c0184e61ebd162ca856d8f61f8195f2405b8 to 6159c1ca2b75e3234e0042bfb9f1770299ab6200
Branch pushed to git repo; I updated commit sha1. New commits:
6159c1c  better unicode support

comment:60 Changed 7 years ago by
 Status changed from needs_review to needs_work
Two things (see patchbot report)
 please use python3style raise statements:
raise Error("this is the good way")
instead of
raise Error, 'this was the old way'
The bot says
Oldstyle raise statement inserted on 5 nonempty lines
 please make sure that every function is doctested.
'Missing doctests ', 'databases/findstat.py', '40 / 57 = 70%')
comment:61 Changed 7 years ago by
 Commit changed from 6159c1ca2b75e3234e0042bfb9f1770299ab6200 to 72b6e380856116d6757fa5179b5f698606223701
Branch pushed to git repo; I updated commit sha1. New commits:
72b6e38  replace oldstyle raise statements

comment:62 followup: ↓ 65 Changed 7 years ago by
Thank you for raising my awareness!
Two questions:
1) where can I find the patchbot report?
2) how am I supposed to doctest functions that use a webbrowser?
comment:63 Changed 7 years ago by
1) Patchbot reports are available by clicking on the round colored icon (top right of the page)
2) I do not know, sorry. I have not looked at your code.
comment:64 Changed 7 years ago by
1) great! Thank you!
2) So, I guess I have to ask on sagedevel...
comment:65 in reply to: ↑ 62 Changed 7 years ago by
Replying to mantepse:
2) how am I supposed to doctest functions that use a webbrowser?
Just have a look at the oeis module. Though I would not say that doctesting all functions is a requirement.
comment:66 Changed 7 years ago by
Thanks Frederic and Vincent! I just looked at
sage coverage findstat.py
which more or less tells me what to do. I guess I'll simply cook up some tests this afternoon.
comment:67 Changed 7 years ago by
 Commit changed from 72b6e380856116d6757fa5179b5f698606223701 to 0abffb86d9bd89c44d5903ba0534c0f3df3cf698
Branch pushed to git repo; I updated commit sha1. New commits:
0abffb8  documentation and doctest fixes

comment:68 Changed 7 years ago by
 Commit changed from 0abffb86d9bd89c44d5903ba0534c0f3df3cf698 to 2ed6389647ca2b9752f38d733f1922323cdb4a09
Branch pushed to git repo; I updated commit sha1. New commits:
2ed6389  more docfixes

comment:69 Changed 7 years ago by
 Commit changed from 2ed6389647ca2b9752f38d733f1922323cdb4a09 to 1d93a015afb7d92a59559b12f132b212298ca40f
Branch pushed to git repo; I updated commit sha1. New commits:
1d93a01  yet more doctests

comment:70 Changed 7 years ago by
 Commit changed from 1d93a015afb7d92a59559b12f132b212298ca40f to 238cd2c8ce6cfa77ffda487ac81c4c9b4dad1c95
Branch pushed to git repo; I updated commit sha1. New commits:
238cd2c  fix problem with determining whether an object is used in a FindStat search

comment:71 Changed 7 years ago by
 Status changed from needs_work to needs_review
comment:72 Changed 7 years ago by
I cannot think of any more sensible tests...
comment:73 Changed 7 years ago by
 Milestone changed from sage6.6 to sage6.7
comment:74 Changed 7 years ago by
 Commit changed from 238cd2c8ce6cfa77ffda487ac81c4c9b4dad1c95 to f89c50a79f9687f9608940ba6fa5ecd7f95ab6be
Branch pushed to git repo; I updated commit sha1. New commits:
f89c50a  even more docfixes

comment:75 Changed 7 years ago by
 Commit changed from f89c50a79f9687f9608940ba6fa5ecd7f95ab6be to f8393b67bb1a3592b392835715df647893cf3b0c
Branch pushed to git repo; I updated commit sha1. New commits:
f8393b6  example demonstrating a common pitfall

comment:76 Changed 7 years ago by
 Commit changed from f8393b67bb1a3592b392835715df647893cf3b0c to ef1def71af41d9efb0fad4e265e850fe3a936ceb
Branch pushed to git repo; I updated commit sha1. New commits:
ef1def7  docfix

comment:77 Changed 7 years ago by
Please avoid using # in the doctests, as for example here:
# when one of these tests does not pass, there is probably a new collection to be added.
comment:78 Changed 7 years ago by
 Commit changed from ef1def71af41d9efb0fad4e265e850fe3a936ceb to 7456e56c019b1c0f6637ef3d75d53caf990fda4a
comment:79 Changed 7 years ago by
 Commit changed from 7456e56c019b1c0f6637ef3d75d53caf990fda4a to 9252e3b9fe5b36f522e3fa2069107dd4bd032caa
Branch pushed to git repo; I updated commit sha1. New commits:
9252e3b  don't even try to modify query results with perfect match

comment:80 Changed 7 years ago by
 Commit changed from 9252e3b9fe5b36f522e3fa2069107dd4bd032caa to d025f46445e6ededdbe640eea05772ade8349c61
Branch pushed to git repo; I updated commit sha1. New commits:
d025f46  try to make patchbot happier

comment:81 Changed 7 years ago by
I just did
sage: A = findstat({ S: len([B for B in S if len(B) == 1 ] ) for S in SetPartitions(4) },depth=0) sage: A.submit() ValueError Traceback (most recent call last) ... ValueError: too many values to unpack
Instead, this worked:
sage: def statistic(S): return len([B for B in S if len(B) == 1 ]) sage: A = findstat(statistic,SetPartitions,depth=0) sage: A.submit()
It took me a few minutes to figure how to properly give a statistic in order to submit it  wouldn't it be nice if the first version would also work, or at least give a hint on how to do it?
comment:82 Changed 7 years ago by
Are you sure you have the most recent version? I get:
sage: A = findstat({ S: len([B for B in S if len(B) == 1 ] ) for S in SetPartitions(4) },depth=0) sage: A 0: (St000247: The number of singleton blocks of a set partition., [], 15) sage: A.submit() ValueError: Your input data matches St000247. Consider modifying this statistic instead.
comment:83 Changed 7 years ago by
haha  I used the second version, submitted the statistic, viviane reviewed it, and you now find it from the interface...
comment:84 Changed 7 years ago by
Oh. I'm about to fix this! Thanks for the bug report!
comment:85 Changed 7 years ago by
 Commit changed from d025f46445e6ededdbe640eea05772ade8349c61 to 4c5a8dcff3fe42d73f67a21e1a2c3a7c0871038e
comment:86 Changed 7 years ago by
 Commit changed from 4c5a8dcff3fe42d73f67a21e1a2c3a7c0871038e to 01f52d5e01420acb3445f2be050b5fb1f8253a48
Branch pushed to git repo; I updated commit sha1. New commits:
01f52d5  fix typo

comment:87 Changed 7 years ago by
Done! Thanks again!
comment:88 Changed 7 years ago by
 Commit changed from 01f52d5e01420acb3445f2be050b5fb1f8253a48 to b633bd3558e456e0d756f4af9d8040c431ad231f
Branch pushed to git repo; I updated commit sha1. New commits:
b633bd3  Merge branch 'develop' into public/findstatinterface

comment:89 Changed 7 years ago by
 Commit changed from b633bd3558e456e0d756f4af9d8040c431ad231f to 800182391f9e34662f8f8b8fbfde275eb5390468
comment:90 Changed 7 years ago by
 Reviewers set to Christian Stump, Viviane Pons
I've read through the whole ticket and documentation: everything seems fine. I've fixed the few little things I've noticed. It seems like everything that had been requested has been done. The design is very reasonable and the tests pass!
So, if everyone's happy I would suggest that Christian or Martin review my latest push and that we put a positive review.
Thank you Martin, I find it a very nice work.
comment:91 Changed 7 years ago by
Great, seems good to me, but I let Martin to have the last check on your changes.
comment:92 Changed 7 years ago by
Wow, great!
Because of lack of a computer I cannot really test your changes, so please forgive if I could answer the following questions myself:
 does it really make sense to make the class
FindStat
inherit fromSageObject
? I thought the rule was for classes with "mathematical" objects, or at least objects the user can actually manipulate or inspect. The reason I did not use it was that the classFindStat
actually should not produce any objects...
 I think I recall that in
__eq__
if (not isinstance(other, FindStatStatistic)): return False
is actually unnecessary  but I'm not sure. Could you please doublecheck? (same forFindStatMap
)
In any case: yes, I'm very happy with having this finished!
Thank you so much!
Martin
comment:93 Changed 7 years ago by
You're right about SageObject? it might not be necessary. I was under the impression that the FindStat? class did return an object but maybe the user can never see this object...
About the eq I did check and it seemed necessary: when I was comparing statisctics or whatever to other things (like integers) I would get an ugly Exception instead of just False. Maybe there is another way to do this. I was actually under the impression that it should have been taken care for by SageObject? but it doesn't seem so...
comment:94 Changed 7 years ago by
Great!
comment:95 Changed 7 years ago by
Hi Viviane!
I have just created two more classes FindStatCollections
and FindStatMaps
which serve as parents for collections and maps. Although nice, this is not a very important feature, I think. So I don't want to delay the review because of that.
However, if you are willing to review this in the same ticket I would push it to trac! (Note that I'm a bit unsure whether I did things correctly with all this classcall magic.)
comment:96 Changed 7 years ago by
 Commit changed from 800182391f9e34662f8f8b8fbfde275eb5390468 to dd50895ccfdcdde5b86595d84db990fe06a42bac
comment:97 Changed 7 years ago by
Sorry Viviane, it's so much better that I pushed. I hope you can give it your OK :)
comment:98 Changed 7 years ago by
 Commit changed from dd50895ccfdcdde5b86595d84db990fe06a42bac to dc6ff91e7a96966a2485cdf61c30cce9dfc513ac
Branch pushed to git repo; I updated commit sha1. New commits:
dc6ff91  provide pickling for maps and collections

comment:99 Changed 7 years ago by
No problem. I ave still one week of sage days so I will have plenty of time to review. And you're right, it's probably better with proper parents.
comment:100 Changed 7 years ago by
 Component changed from interfaces to interfaces: optional
comment:101 Changed 7 years ago by
Hi Viviane 
I wanted to check what the status of the review here is... also: what is the difference between interfaces
and interfaces: optional
Christian
comment:102 Changed 7 years ago by
 Commit changed from dc6ff91e7a96966a2485cdf61c30cce9dfc513ac to 8777b30330c5cf5f02a4a6c69669ff089a76ee63
Branch pushed to git repo; I updated commit sha1. New commits:
8777b30  correct and improve error handling

comment:103 Changed 7 years ago by
I just noticed that the interface is slightly inconsistent: it expects either objects and values, or a statistic in form of a callable and a collection. I guess the collection should come first.
comment:104 Changed 7 years ago by
 Commit changed from 8777b30330c5cf5f02a4a6c69669ff089a76ee63 to ffc90a85672c745f6fad2a20b6e18b20002e598b
Branch pushed to git repo; I updated commit sha1. New commits:
ffc90a8  change order of arguments when providing a statistic in form of a callable

comment:105 Changed 7 years ago by
 Status changed from needs_review to needs_work
does not build, see patchbot report (metaclass problem)
comment:106 Changed 7 years ago by
 Commit changed from ffc90a85672c745f6fad2a20b6e18b20002e598b to e727d044735ce75ff917dc92e7532dd20d48346d
comment:107 Changed 7 years ago by
I have no idea why, but it seems that I needed to replace
__metaclass__ = ClasscallMetaclass
with
__metaclass__ = InheritComparisonClasscallMetaclass
Just took me a few hours :(
comment:108 Changed 7 years ago by
 Status changed from needs_work to needs_review
comment:109 Changed 7 years ago by
The patchbot says that there are two missing doctests. Please recheck!
comment:110 Changed 7 years ago by
 Commit changed from e727d044735ce75ff917dc92e7532dd20d48346d to 482caecc0ac94237a8b8053b6418ecb6fd8eedce
Branch pushed to git repo; I updated commit sha1. New commits:
482caec  provide doctests to make the patchbot happy, improve documentation

comment:111 Changed 7 years ago by
 Commit changed from 482caecc0ac94237a8b8053b6418ecb6fd8eedce to 21c4812b58991aff37914c2948d7409151ea1ed7
Branch pushed to git repo; I updated commit sha1. New commits:
21c4812  Merge branch 'develop' into public/findstatinterface

comment:112 Changed 7 years ago by
 Commit changed from 21c4812b58991aff37914c2948d7409151ea1ed7 to f841404abb56f2a7629e373b198e1482c84a5285
Branch pushed to git repo; I updated commit sha1. New commits:
f841404  moved a few things in the documentation

comment:113 Changed 7 years ago by
I have been playing with the interface for some time now, and I consider it to be ready to get merged. Martin did a great job and the "guided tour" is an awesome point of entry to use the interface, with several easy examples to follow! When they pop up in real life, it might be worth to add also some more sophisticated use cases.
All tests pass on my machine with the optional flags set, I still wait for the patchbot to be green again (though it does not do any doctests since they all require an internet connection) to give it a positive review.
comment:114 Changed 7 years ago by
 Status changed from needs_review to positive_review
comment:115 Changed 7 years ago by
 Status changed from positive_review to needs_work
Attempts internet access:
sage t long src/sage/databases/findstat.py ********************************************************************** File "src/sage/databases/findstat.py", line 844, in sage.databases.findstat.FindStatStatistic._find_by_id Failed example: findstat(999999) # indirect doctest Expected: Traceback (most recent call last): ... ValueError: St999999 is not a FindStat statistic identifier. Got: <BLANKLINE> Traceback (most recent call last): File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 496, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 858, in compile_and_execute exec(compiled, globs) File "<doctest sage.databases.findstat.FindStatStatistic._find_by_id[0]>", line 1, in <module> findstat(Integer(999999)) # indirect doctest File "sage/misc/lazy_import.pyx", line 383, in sage.misc.lazy_import.LazyImport.__call__ (/Users/buildslavesage/slave/sage_git/build/src/build/cythonized/sage/misc/lazy_import.c:3593) return self._get_object()(*args, **kwds) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/databases/findstat.py", line 476, in __call__ return self._statistic_find_by_id_cached(query) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/databases/findstat.py", line 631, in _statistic_find_by_id_cached self._statistic_cache[id] = FindStatStatistic(id)._find_by_id() File "/Users/buildslavesage/slave/sage_git/build/local/lib/python2.7/sitepackages/sage/databases/findstat.py", line 855, in _find_by_id self._raw = json.load(urlopen(url), object_pairs_hook=OrderedDict) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 154, in urlopen return opener.open(url, data, timeout) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 431, in open response = self._open(req, data) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 449, in _open '_open', req) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 409, in _call_chain result = func(*args) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 1227, in http_open return self.do_open(httplib.HTTPConnection, req) File "/Users/buildslavesage/slave/sage_git/build/local/lib/python/urllib2.py", line 1197, in do_open raise URLError(err) URLError: <urlopen error [Errno 60] Operation timed out> ********************************************************************** 1 item had failures: 1 of 2 in sage.databases.findstat.FindStatStatistic._find_by_id [50 tests, 1 failure, 75.18 s]
comment:116 Changed 7 years ago by
 Status changed from needs_work to needs_review
Isn't that part of the nature of an internet access that a connection can time out? That also happened to me before, but when rerunning the tests, everything went well. How should doctests that need an internet connection be handled otherwise?
comment:117 Changed 7 years ago by
 Commit changed from f841404abb56f2a7629e373b198e1482c84a5285 to ec3bc2abce27759168e9d6ca5d3cb4a38a7dde4b
Branch pushed to git repo; I updated commit sha1. New commits:
ec3bc2a  added the optional flag to one test

comment:118 Changed 7 years ago by
Sorry, I think the problem was that this test was missing the flag optional  internet
, that's why you got the doctest failure. Was that is?
New commits:
ec3bc2a  added the optional flag to one test

comment:119 Changed 7 years ago by
Martin, would you briefly look at my change, do the doctests, and set the ticket back to positive review. Thanks!
comment:120 Changed 7 years ago by
 Status changed from needs_review to positive_review
Well, the change is trivial  I only didn't know that I'm entitled to "positively review" it...
(Besides, I do not understand why you moved the definitions of collection, map and statistic, but I admit that I do not really care... In the same commit, there is also a typo: "entriy" should by "entry". Apart from that, I agree that the changes are an improvement! Thanks!)
comment:121 Changed 7 years ago by
 Branch changed from public/findstatinterface to ec3bc2abce27759168e9d6ca5d3cb4a38a7dde4b
 Resolution set to fixed
 Status changed from positive_review to closed
I know this isn't yet set to
needs_review
, but the documentation is misformatted in many places. It should be like