maxpumperla / betago

BetaGo: AlphaGo for the masses, live on GitHub.
http://maxpumperla.github.io/betago
MIT License
680 stars 162 forks source link

Python 3 #35

Closed hadim closed 7 years ago

hadim commented 7 years ago

Is it Python 3 compatible ?

from betago.processor import SevenPlaneProcessor
processor = SevenPlaneProcessor()
input_channels = processor.num_planes

# Load go data and one-hot encode labels
X, y = processor.load_go_data(num_samples=1000)
X = X.astype('float32')
Y = np_utils.to_categorical(y, nb_classes)

raises

AttributeError                            Traceback (most recent call last)
<ipython-input-4-5682cb2fef10> in <module>()
      4 
      5 # Load go data and one-hot encode labels
----> 6 X, y = processor.load_go_data(num_samples=1000)
      7 X = X.astype('float32')
      8 Y = np_utils.to_categorical(y, nb_classes)

/home/hadim/Insync/Documents/Code/misc/go_game/betago/betago/dataloader/base_processor.py in load_go_data(self, types, data_dir, num_samples)
    119         num_samples: Number of Go games to load.
    120         '''
--> 121         index = KGSIndex(data_directory=self.data_dir)
    122         index.download_files()
    123 

/home/hadim/Insync/Documents/Code/misc/go_game/betago/betago/dataloader/index_processor.py in __init__(self, kgs_url, index_page, data_directory)
     41         self.file_info = []
     42         self.urls = []
---> 43         self.load_index()  # Load index on creation
     44 
     45     def download_files(self):

/home/hadim/Insync/Documents/Code/misc/go_game/betago/betago/dataloader/index_processor.py in load_index(self)
     94         Create the actual index representation from the previously downloaded or cached html.
     95         '''
---> 96         index_contents = self.create_index_page()
     97         split_page = [item for item in index_contents.split('<a href="') if item.startswith("https://")]
     98         for item in split_page:

/home/hadim/Insync/Documents/Code/misc/go_game/betago/betago/dataloader/index_processor.py in create_index_page(self)
     81         else:
     82             print('>>> Downloading index page')
---> 83             fp = urllib.urlopen(self.kgs_url)
     84             data = unicode(fp.read())
     85             fp.close()

AttributeError: module 'urllib' has no attribute 'urlopen'
hadim commented 7 years ago

I use Python 3.6

hadim commented 7 years ago

Here is the patch that used to make it work :

diff --git a/betago/dataloader/index_processor.py b/betago/dataloader/index_processor.py
index 89effe0..0bc8f54 100644
--- a/betago/dataloader/index_processor.py
+++ b/betago/dataloader/index_processor.py
@@ -4,9 +4,13 @@
 from __future__ import print_function
 import os
 import sys
-import urllib
 import multiprocessing

+if sys.version_info[0] == 3:
+    from urllib.request import urlopen, urlretrieve
+else:
+    from urllib import urlopen, urlretrieve
+

 def worker(url_and_target):
     '''
@@ -15,7 +19,7 @@ def worker(url_and_target):
     try:
         (url, target_path) = url_and_target
         print('>>> Downloading ' + target_path)
-        urllib.urlretrieve(url, target_path)
+        urlretrieve(url, target_path)
     except (KeyboardInterrupt, SystemExit):
         print('>>> Exiting child process')

@@ -80,8 +84,8 @@ class KGSIndex(object):
             index_file.close()
         else:
             print('>>> Downloading index page')
-            fp = urllib.urlopen(self.kgs_url)
-            data = unicode(fp.read())
+            fp = urlopen(self.kgs_url)
+            data = str(fp.read())
             fp.close()
             index_contents = data
             index_file = open(self.index_page, 'w')
hadim commented 7 years ago

Also the tutorial lacks some import to work out of the box (related to Keras such as layers and Sequential).

macfergus commented 7 years ago

Hi hadim, betago theoretically supports python 3, but as you can see we haven't been careful about testing in both Python versions.

Thanks for your patch! I will modify it to use the six library and merge.

maxpumperla commented 7 years ago

above PR will hopefully close this. we still need more test coverage for the data loading.

maxpumperla commented 7 years ago

About the readme, it's meant to skim through to get an idea, but maybe we should link to a full example explicitly.

maxpumperla commented 7 years ago

alright @hadim, I'll close this for now. Thanks again for the patch! let me know in case we missed something.