opencog / atomspace

The OpenCog (hyper-)graph database and graph rewriting system
https://wiki.opencog.org/w/AtomSpace
Other
813 stars 225 forks source link

Python: initialize, finalize opencog not needed #2411

Open linas opened 4 years ago

linas commented 4 years ago

In various places, e.g. tests/cython/bindlink/test_bindlink.py we see this python code:

from opencog.utilities import initialize_opencog, finalize_opencog

These are not needed; they should be called automatically, when from opencog.atomspace import * is called.

So for example, farther down in that file:

        # Initialize Python
        initialize_opencog(self.atomspace)
        set_type_ctor_atomspace(self.atomspace)

Since python no longer has a single, global atomspace, the call to initialize_opencog(self.atomspace) is pointless. Only the type_ctor thing is needed.... although even that could be automated. When one does the atomspace = AtomSpace() it should automatically set the type_ctr_atomspace.

This is a followup to discussions in pull req #2398

linas commented 4 years ago

@noskill @vsbogd followup to #2398

vsbogd commented 4 years ago

Python still has single global atomspace, which is used to call type constructors without specifying atomspace explicitly: https://github.com/opencog/atomspace/blob/2c99e42c9b7f6141da942853f92ebfb39c1e642f/opencog/cython/opencog/type_constructors.pyx#L14-L21

2398 removes second global variable which pretended to be a global atomspace in Python but in fact was not used anywhere.

linas commented 4 years ago

why did you close this? Of the two functions, only one is needed, both cannot possibly be needed.

linas commented 4 years ago

This is almost finished, thanks to pull req #2416

ntoxeg commented 4 years ago

These are not needed; they should be called automatically, when from opencog.atomspace import * is called.

If we follow through with the changes I've made in #2550, you shouldn't do from opencog.atomspace import * - and generally this is bad Python practice. In this case there is name pollution because it imports TruthValue and you should use the TruthValue from type_constructors.

Either importing everything from opencog.atomspace should be discouraged, or one can simply add __all__ list in the module to limit what gets exported when doing a * import.

ntoxeg commented 4 years ago

What is actually left to be done in this issue?

linas commented 4 years ago

What is actually left to be done in this issue?

cd tests
grep -r initialize_opencog
grep -r finalize_opencog

There are also likely users in other github repos...