These are chat archives for bluescarni/pagmo_reborn

1st
Jul 2016
Dario Izzo
@darioizzo
Jul 01 2016 08:47
We need to decide how to deal with the python import maddness. I have created the first pure python file named special_plot.py. It contains a function called plot_pareto_fronts. Where do we want that to appear?
A module? or the root namespace? In this second case how do we achieve it making sure that both from pygmo import * and import pygmo make sense?
@bluescarni
Francesco Biscani
@bluescarni
Jul 01 2016 08:49
IMO it's not so much deciding, it's about reading up about best practices and learning about the differences
Dario Izzo
@darioizzo
Jul 01 2016 08:49
any particular site to read this from? I found different opinions
so the important points as far as I am concerned:
  • both proper programming use and interactive use should be possible
  • probably use python3 relative imports inside init.py?
  • make sure the sphinx docs pick up the correct submodule when generating the docs
what I mean by the third point is that in my experience sphinx considers a class "belonging" into the module in which it was originally defined
that's way for instance we have to document exposed C++ stuff from pygmo.core
cause that's where the classes are originally defined and we cannot change that
Dario Izzo
@darioizzo
Jul 01 2016 08:55
we can only make them appear in the root namespace ..
Francesco Biscani
@bluescarni
Jul 01 2016 08:56
probably in a pure-python module the classes imported in __init__.py figure as belonging to that module? not sure
we can make them appear in pygmo. but is we ask sphinx to document, e.g., pygmo.population it will not work
Dario Izzo
@darioizzo
Jul 01 2016 08:57
yep
Francesco Biscani
@bluescarni
Jul 01 2016 08:57
try to come up with an initial structure, then we can review it
Dario Izzo
@darioizzo
Jul 01 2016 08:57
k
reading
Dario Izzo
@darioizzo
Jul 01 2016 09:09
I think we need to create one folder per module (core, test) and put there some __init__, in any case I am super confused :)
Francesco Biscani
@bluescarni
Jul 01 2016 09:09
the core is pure C++, we don't need an init for that I think
Dario Izzo
@darioizzo
Jul 01 2016 09:10
Yes, I am saying for good practices / consistency
Francesco Biscani
@bluescarni
Jul 01 2016 09:10
I think it will break sphinx, the classes will come out as pygmo.core.core.population etc.
I believe at least
Dario Izzo
@darioizzo
Jul 01 2016 09:11
uff .. ok I will make some trials
Francesco Biscani
@bluescarni
Jul 01 2016 09:11
I'd focus first on the pure python bits at first, the compiled stuff is a bit more esoteric
Dario Izzo
@darioizzo
Jul 01 2016 09:12
And we still do not want modules right? We want all in the root namespace. Like pygmo.plot_archi etc. should be possible as well as pygmo.special_plots.plot_archi
?
Francesco Biscani
@bluescarni
Jul 01 2016 09:13
AFAIK, there's a place where functions are defined which is immutable (and shows up in sphinx), and then you can make "links" to any entity from anywhere, for convenience
I think that was our idea? define it in its own module, then create a link in the root namespace
for tab completion mostly
as far as I remember
Francesco Biscani
@bluescarni
Jul 01 2016 09:19
let's just try to get this right on the first try, because doing it randomly and putting FIXMEs and TODOs won't work :)
Dario Izzo
@darioizzo
Jul 01 2016 09:58
Agreed. I pushed a proposal
It would be the template for all pure python stuff (I ignored the cpp part). It contains a new "module" called pygmo_plots that is also imported on the root namespace.
@bluescarni
I am now testing the sphinx behaviour
Francesco Biscani
@bluescarni
Jul 01 2016 10:03
so what's the rationale of adding all the stuff in the module to the __all__ variable?
otherwise from pygmo import * imports the submodule name but not all its members?
from pygmo.pygmo_plots import * I think this should use relative imports: from .pygmo_plots import *, if I recall the syntax correctly
Francesco Biscani
@bluescarni
Jul 01 2016 10:09
This message was deleted
Francesco Biscani
@bluescarni
Jul 01 2016 10:21
name.startswith('__') I'd replace this with name.startswith('_'), I don't think the protected members need to be imported in the root namespace
Dario Izzo
@darioizzo
Jul 01 2016 10:59
k. I though about it, but then for some reason decided to treat protected members as normal functions as they indeed are only a convention (the __ ones at least have a different behavior defined from python point of view)
Francesco Biscani
@bluescarni
Jul 01 2016 11:00
right. ipython also does not show them in tab completion
it the adding of all members to __all__ necessary?
Dario Izzo
@darioizzo
Jul 01 2016 11:18
You mean the
# For convenience import core the root namespace
from pygmo.core import *
__all__ += [name for name in dir(core) if not name.startswith('_')]

# For convenience import plotting functions into the root namespace
from pygmo.pygmo_plots import *
__all__ += [name for name in dir(pygmo_plots) if not name.startswith('_')]
Francesco Biscani
@bluescarni
Jul 01 2016 11:18
yes
Dario Izzo
@darioizzo
Jul 01 2016 11:18
If we do it, then from pygmo import * has the behaviour we agreed
otherwise tyou will not find in the root namespace all the algos, probs etc.
Francesco Biscani
@bluescarni
Jul 01 2016 11:19
but then wouldn't it just be better to remove the __all__ variable if we are going to put everything in it anyway?
Dario Izzo
@darioizzo
Jul 01 2016 11:19
The other way (more correct) is to from pygmo.algorithms import * etc...
I thought its necessary .. let me try to remove it
Francesco Biscani
@bluescarni
Jul 01 2016 11:19
and those sould be relative imports
Dario Izzo
@darioizzo
Jul 01 2016 11:20
?
Francesco Biscani
@bluescarni
Jul 01 2016 11:20
from .pygmo_plots import *
Dario Izzo
@darioizzo
Jul 01 2016 11:21
whats the diff? do I want to know?
Francesco Biscani
@bluescarni
Jul 01 2016 11:21
with a relative import there's no doubt you are asking to load the module from the current directory
plus you avoid repeating the name of the supermodule
Dario Izzo
@darioizzo
Jul 01 2016 11:22
so commenting out the lines:

__all__ = ['core', 'test', 'pygmo_plots']
# For convenience import core the root namespace
from .core import *
__all__ += [name for name in dir(core) if not name.startswith('_')]

# For convenience import plotting functions into the root namespace
from .pygmo_plots import *
__all__ += [name for name in dir(pygmo_plots) if not name.startswith('_')]
all still works as expected. But we loose (I guess) the possibility in the future to be selective?
maybe we do not care ... lets not overdesign
I delete it ok?
Francesco Biscani
@bluescarni
Jul 01 2016 11:24
the __all__ you mean?
Dario Izzo
@darioizzo
Jul 01 2016 11:24
yes
I do not define __all__
so by default he gets all :)
Francesco Biscani
@bluescarni
Jul 01 2016 11:24
ok but then you also have to add the import of the modules... like this:
from . import core, pygmo_plots

from .core import *
from .pygmo_plots import *
I think... so you have pygmo.core.population and pygmo.population both
Dario Izzo
@darioizzo
Jul 01 2016 11:25
i still have it
without your lines
mmm
Francesco Biscani
@bluescarni
Jul 01 2016 11:26
that sounds strange
what happens if you do pygmo.core? in ipython?
Dario Izzo
@darioizzo
Jul 01 2016 11:26
testing
ok you are right (as usual)
there was the line from .core import * hidden later
I moved it on top
so which way we go? no __all__ def + imports?
Francesco Biscani
@bluescarni
Jul 01 2016 11:28
is it really necessary to call it pygmo_plots? pygmo.pygmo_plots? why not pygmo.plots?
Dario Izzo
@darioizzo
Jul 01 2016 11:28
because then its imported in the root namespace as plots
if you do from pygmo import *
I thought the idea was to have everything also in the root namespace correct?
I also do not like the name. But its the only one I could think of that does not pollute the namespace special_plots was also an option
Francesco Biscani
@bluescarni
Jul 01 2016 11:31
maybe plotting
?
I mean the * import is gonna have name clashes with everything eventually
Dario Izzo
@darioizzo
Jul 01 2016 11:32
could be ok ...
Francesco Biscani
@bluescarni
Jul 01 2016 11:32
if you think how much stuff matplotlib, numpy and scipy export
Dario Izzo
@darioizzo
Jul 01 2016 11:32
i mean the name plotting
Francesco Biscani
@bluescarni
Jul 01 2016 11:34
we gonna have a name called de in the global namespace after a * import
not exactly foolproof :)
Dario Izzo
@darioizzo
Jul 01 2016 11:34
for real
How do we deal with third party extesion required (numpy and matplotlib)? I vote for just assuming they are installed
Francesco Biscani
@bluescarni
Jul 01 2016 11:39
numpy is a prerequisite for compilation, cmake will fail if it is not found
Dario Izzo
@darioizzo
Jul 01 2016 11:39
for matplotlib do we want to detect it upon import of pygmo or at compile time?
Francesco Biscani
@bluescarni
Jul 01 2016 11:40
the way I see it, it's a runtime dependency, so it must not be checked at compile time
also, in the case of matplotlib, if it is not available you just lose 1 function so far
Dario Izzo
@darioizzo
Jul 01 2016 11:40
so the behaviour we want is: if its there plotting is imported, otherwise it still works bu the module does not exist?
Francesco Biscani
@bluescarni
Jul 01 2016 11:40
so I would not make it a required deps
without doing anything special, you'll get an import error... which might be good enough
Dario Izzo
@darioizzo
Jul 01 2016 11:41
ok but this forces us to put all import in the function body.
is that good practice?
Francesco Biscani
@bluescarni
Jul 01 2016 11:42
it's better practice, otherwise it's using namespace std; all over again
if you import it from the module, you will get a matplotlib submodule in the plotting module
     from matplotlib import pyplot as plt
     from pygmo.core import fast_non_dominated_sorting, population

     from numpy import linspace
also here it should be a relative import
from ..core import ...
in general all stuff belonging to the module itself shuold be imported in a relative fashion, absolute imports only for external packages
and put from __future__ import absolute_import as _ai at the top of the plotting module for Python 2 compatibility
Dario Izzo
@darioizzo
Jul 01 2016 11:47
PEP8 says import should be at the top of the files .....
Francesco Biscani
@bluescarni
Jul 01 2016 11:47
well we actually intendo to support only python 2.7 I believe so not sure it's really necessary
Dario Izzo
@darioizzo
Jul 01 2016 11:47
But I agree with you, I do not like plt to be in the namespace ....
Francesco Biscani
@bluescarni
Jul 01 2016 11:48
in module files it should be at the top, like our imports in __init__.py
that's how I interpret it, not as in "put your imports at the module level rather than in the functions"
Dario Izzo
@darioizzo
Jul 01 2016 13:50
In [1]: from pygmo import *

In [2]: prob = zdt()

In [3]: pop = population(prob,40)

In [4]: ax = plot_non_dominated_fronts(pop.get_f())
figure_1.png
Francesco Biscani
@bluescarni
Jul 01 2016 13:51
pretty
Dario Izzo
@darioizzo
Jul 01 2016 13:52
then:
Francesco Biscani
@bluescarni
Jul 01 2016 13:52
what is points? a 2d array or list of arrays?
Dario Izzo
@darioizzo
Jul 01 2016 13:52
In [9]: algo  =algorithm(moead(500))

In [10]: pop = algo.evolve(pop)

In [11]: ax = plot_non_dominated_fronts(pop.get_f())
figure_1-1.png
from the docs:
points (array [or list] of arrays [or lists of doubles): points to plot
Francesco Biscani
@bluescarni
Jul 01 2016 13:53
right. I was just wondering if you could use a numpy syntax instead of [13][0]
Dario Izzo
@darioizzo
Jul 01 2016 13:53
right .. I think there was some pasting problem wait
not sure what that was ....
changed it
Francesco Biscani
@bluescarni
Jul 01 2016 13:55
cool ok
Dario Izzo
@darioizzo
Jul 01 2016 13:56
I am finished ... I will push. Its pretty a lot to review, but i think its important to spend some time on it .... main issues are on how we want the docs to appear (I know you love this point) and incostistencies ...
plus bugs and cagnate
Francesco Biscani
@bluescarni
Jul 01 2016 13:57
what do you mean inconsistencies?
Dario Izzo
@darioizzo
Jul 01 2016 13:58
Well, for example, how do we want a function to appear on sphinx? right now its:
Screenshot from 2016-07-01 15-57-44.png
We are doubling the arg list, it appears two times.
Francesco Biscani
@bluescarni
Jul 01 2016 13:58
why is that? it's not like that in any other doc bit
Dario Izzo
@darioizzo
Jul 01 2016 13:59
Because I put it :) I though it could be a good idea, but I can remove it.
It may be useful to resolve overloads
Francesco Biscani
@bluescarni
Jul 01 2016 13:59
I don't understand... isn't that a pure python function?
Dario Izzo
@darioizzo
Jul 01 2016 13:59
yes
Francesco Biscani
@bluescarni
Jul 01 2016 14:00
so there's no overload possible in python
Dario Izzo
@darioizzo
Jul 01 2016 14:00
but in other case (when it will be from c++) we will have 2 overloads possible. In which case I think sphinx needs the signature which repeats on the screen
Francesco Biscani
@bluescarni
Jul 01 2016 14:01
I have so far managed to avoid overloads "hiding" them in functions with default parameters
for instance in the various constructors
Dario Izzo
@darioizzo
Jul 01 2016 14:01
ok so I remove, and well see
Francesco Biscani
@bluescarni
Jul 01 2016 14:01
which are overloaded in C++ but show up as a single constructor
we should actually actively try to avoid such a situation
Dario Izzo
@darioizzo
Jul 01 2016 14:02
and the return type from matplotlib stuff? is the thing returned by type(obj) OK?
Francesco Biscani
@bluescarni
Jul 01 2016 14:02
the doc workflow situation is already bad as it is
isn't the return type/value documented below?
in python it does not make much sense to document the return type in the signature, as it does not appear there
Dario Izzo
@darioizzo
Jul 01 2016 14:03
Yes I mean at the moment is : matplotlib.axes._subplots.AxesSubplot
as its what get_gca() returns ... is it ok?
do we put the plt.show() in the function? it halts in non interactive mode the flow.
Francesco Biscani
@bluescarni
Jul 01 2016 14:04
the presence of an underscore in that name makes me think it is not the "public" definition of that object, so that might eventually change in other matplotlib versions
Dario Izzo
@darioizzo
Jul 01 2016 14:04
and all these detials ....
need to go sorry ....
Francesco Biscani
@bluescarni
Jul 01 2016 14:08
anyway since you asked: the markup of the return type should be consistent with the markup of types above (float, str) and there is a missing closing square bracket in the documentation of the points argument
comp is marked a float but the default value is a list...
Francesco Biscani
@bluescarni
Jul 01 2016 14:17
did you also change the return type of those integer arrays to signed?
Dario Izzo
@darioizzo
Jul 01 2016 18:37
hey, on the markup, I do not get if we want all types to be marked-up, that is appearing as floats, str, int or that is only to be done on basic types.
I mean:
matplotlib.axes._subplots.AxesSubplot
this goes like int?
and:
list or NumPy array or array [or list] of arrays [or lists] of doubles
all go marked up too?
Francesco Biscani
@bluescarni
Jul 01 2016 18:50
for consistency I'd say all or nothing, though I suppose one could make the argument that "array" is a generic term (and indeed the numpy class is called ndarray, not array)
Dario Izzo
@darioizzo
Jul 01 2016 18:52
Si will do all
Dario Izzo
@darioizzo
Jul 01 2016 19:22
sorry if I ask, I am loosing too much time on this bllst ... if i write
:class:`~pygmo.core.algoritm`
sphinx does not complain, puts a red text, but it does not link to the documented class ...
any idea why? The same works with:
:class:`~pygmo.core.de`
Dario Izzo
@darioizzo
Jul 01 2016 20:12
Some decision I took for consistency (all can be changed, as far as its consistent):
  • we say float, not double in the python docs
  • we write ``list`` of ``tuples`` , not ``list of tuples``
  • complex return types are documented as:
Returns:
    (``tuple``): (*ndf*, *dl*, *dc*, *ndr*)

Where:
    * *ndf* (``list`` of ``arrays``): the non dominated fronts
    * *dl* (``list`` of ``arrays``): the domination list
    * *dc* (``array``): the domination count
    * *ndr* (``array``): the non domination ranks
Still to understand:
  • You put here and there a # doctest: +SKIP what was the intended purpose?
  • See above for the :class:`~pygmo.core.algoritm` issue
  • What to do with the Raises in python? At the moment it is not exhaustive, only the ones directly thrown in the C++ body or python body are inserted (to be made consistent if we agree on this)
Dario Izzo
@darioizzo
Jul 01 2016 20:18
Still to do:
  • Remove more inconsistencies
  • Uniform the old docs (like problem) to the more recent "style"
Francesco Biscani
@bluescarni
Jul 01 2016 20:20
did you write algoritm without the h?
Dario Izzo
@darioizzo
Jul 01 2016 20:23
lol
Francesco Biscani
@bluescarni
Jul 01 2016 20:23
# doctest: +SKIP this has to do with doctesting. basically, you can have the doctest module to automatically run the small snippets of code example we put in the docstrings, and then compare the output of the code with the output that was put in the snippet of code
and if it does not match, an error will be raised. So it's useful to make sure the code examples always work, and they are up to date
that directive tells the doctest module to skip the comparison of the output
I usually use it when there's floating point bullshit or similar stuff involved
and you cannot be sure the output will be exactly as expected
all of this is moot because apparently the doctests stuff ignores boost python exposed functions
but it works for pure python functions