Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for python3 #190

Closed
sl1pkn07 opened this issue Jan 22, 2017 · 15 comments
Closed

Add support for python3 #190

sl1pkn07 opened this issue Jan 22, 2017 · 15 comments
Labels

Comments

@sl1pkn07
Copy link

Hi

is possible add support for python3?

greetings

@amayra
Copy link

amayra commented Jan 22, 2017

is possible to ask why ?

@amayra
Copy link

amayra commented Jan 22, 2017

from py website

Note that Python 3.5+ cannot be used on Windows XP or earlier.

bytheway this why not

@sl1pkn07
Copy link
Author

is possible to ask why ?

what is the real problem make a dual python support like other programs/libs?

@whinette
Copy link
Contributor

It's more a pita to maintain.

@vampiricwulf
Copy link

A really big pain. Python 2 and Python 3 share the same fundamental design, but are very different. You're basically asking a person who does this in their free time to double their work.

@sl1pkn07
Copy link
Author

sl1pkn07 commented Jan 22, 2017

and when python2 reatches its EOL?

like python1

https://www.python.org/dev/peps/pep-0373/

@vampiricwulf
Copy link

When it reaches its EOL, then you update the code to Python 3 and you don't have to manage two separate languages doing the same thing.

@sl1pkn07
Copy link
Author

like now? i'm linux user, need both pythons because need keep older version of python for programs like this

@Nandaka
Copy link
Owner

Nandaka commented Jan 23, 2017

nope, no time to do it. I'm accepting pull request if someone can do it (and it doesn't break anything).

I'm using windows and python 2.7.x only.

@rachmadaniHaryono
Copy link
Contributor

hi, i'm trying on my fork so it can be able to run on python3. until this commit rachmadaniHaryono@edcf06f i have sucessfully download single image id by copy the session value from browser.

here are some difficulties, when trying to port it:

  • i skip setting system default encoding based on this recommendation https://stackoverflow.com/questions/3828723/why-should-we-not-use-sys-setdefaultencodingutf-8-in-a-py-script
  • also skip print_progress. may or may not related to python3 port
  • usage of file keyword. use open to replace it. related stackoverflow https://stackoverflow.com/questions/24942358/is-file-a-keyword-in-python
  • use in instead of has_key to check key on dict
  • i use six to fix some compatibility.
    • because unicode is replace with six.u, some check with isinstance have to be change to six.text_type instead of unicode
    • some module that use this
      • httplib.BadStatusLine
      • urllib2.URLError
      • urllib2.HTTPError
      • reload
      • unichr
      • unicode
      • raw_input
      • htmlparser
      • urllib2
      • ConfigParser
  • there are already config for pylint and pep8, but both have not max-line-length defined, so i just made one based on the max line length i found (210).
  • mechanize
    • mechanize is not available python3, i'm using mechanicalsoup to replace it. i haven't check all the pixivutil feature, but there is already mechanize feature, which is not yet on mechanical soup (see open_novisit below)
    • mechanicalsoup don't have open_novisit method. for temporary fix, i skip the filesize check and use python3's urlopen to get response for Request object.
    • method to replace unenscape_charref is also skipped. not quite sure if the fix also needed on python3 or even mechanicalsoup
    • return value of Browser.open is now quite different. it return requests.models.Response obj.
    • some setting on Browser is skipped, because it is not implemented on mechanicalsoup (set_handle_redirect, set_handle_referer, set_handle_robots, set_debug_http)
  • BeautifulSoup: for this i use BeautifulSoup4 to use it with python3
    • there are some warning, which caused by non existent parser argument
    • there are some change with html tag checking
    • when parsing tag (ParseTags(self, page)), tag['class'] return list instead of string
  • print function is one of the most changed on the program. there are 2 problem with current print usage on the program
  • some misc thing
    • PixivUtil2module
      • PixivArtist is not used
      • ERROR_SUCCESS is imported twice

rachmadaniHaryono referenced this issue Mar 2, 2018
* chg: dev: use python3 print format

* chg: dev: use future lib

* chg: dev: print line with trailing comma

* chg: dev: missing parenthesis

* fix: dev: miss end kwarg
@rachmadaniHaryono
Copy link
Contributor

rachmadaniHaryono commented Mar 20, 2018

finished setting a travis test on test.PixivModel.py

  • it used pytest as test framework
    • it report `test.PixivModel.py as invalid filename.
      • solution: symlink it to test_PixivModel.py on travis
  • on installation phase the config for python3.3 failed because Numpy 1.14.2 require either python2.7 or minimum python3.4
  • py2/py3
    • re.findall don't accept bytes, so solution for using six's unicode not work directly.
      • solution: currently recast it to str on python3
    • re.findall result also can't compared to integer. on python3 it will return list
  • test module (test.PixivModel.py)
    • with python3 which require bs4, there will be warning when no parser is not defined.
      • solution: create new class which inherit BeautifulSoup. current parser is set to html.parser
    • some test is set to xfail. should compare it with python2.7 test result and if possible fix it. current number of xfail is 33 (at the time of writing)

@Nandaka can you check if this branch test result is accurate https://travis-ci.org/rachmadaniHaryono/PixivUtil2/builds/355791687 ?

e: current tag parse method seems to be different on py2/py3.

@Nandaka
Copy link
Owner

Nandaka commented Mar 21, 2018

get the latest (089c171), I've changed some of the test cases, it should be ok, I think

@rachmadaniHaryono
Copy link
Contributor

rachmadaniHaryono commented Mar 21, 2018

I'm not on pc now, but I see you have added Travis config.

Do you want add all test to Travis?

E: also travis test said it is working so I work on it later

e2: test fix merged

  • remove warning when using has_key method by using has_attr on py3
  • six.u can't be used directly on some case. it will raise TypeError: decoding Unicode is not supported.
    • solution: create custom unicode function that will return the input if TypeError raised. this is similar to this https://stackoverflow.com/a/3858057/1766261 except it only try to use six.u instead decode to utf8. it don't fix all problems on unicode function usage, so on some case another simple check/fix added.
      • currently it is put on PixivHelper.py. not quite sure about this, but PixivModel and PixivHelper relationship import look like circular import. it should be noted on future.
  • tried to test the program using tox. create MANIFEST.in to fix some missing file.

e3: related #347

e4:

  • ParseTags method seems to be too hard to adapt into py2/py3 code. copy the py2 method, port it to py3 and put it on its own class method
  • similar thing happen to parseImageBookmark method. the default regex don't match with bs4 (see tag['class'] point above. otherwise with bs4, the regex may not needed anymore.
  • except that certain image id which is not found by above method. the anchor tag with parent ul.image-items not found this tag. see below for actual anchor-tag
  • bs4 also change the logic on when getting image title. see below for result anchor tag. one extra tag can be filtered by it text pixiv. but this will result on error if actual image title is pixiv
  • there is special AttributeError on python3.4 travis https://travis-ci.org/rachmadaniHaryono/PixivUtil2/jobs/356358449 , similar error can be found on this issue https://bugs.python.org/issue23003

2 special anchor tag on parseImageBookmark

<a class="work _work " href="member_illust.php?mode=medium&amp;illust_id=23353599" style="display:block">
  <div class="_layout-thumbnail">
    <img class="ui-scroll-view" data-filter="thumbnail-filter lazy-image" data-id="23353599" data-src="https://source.pixiv.net/common/images/limit_unknown_s.png?20110520" data-tags="----- ----- ----- ----- ----- ----- ----- ----- ----- -----" data-type="illust" data-user-id="0" src="https://source.pixiv.net/www/images/common/transparent.gif"/>
  </div>
</a>
<a href="member_illust.php?mode=medium&amp;illust_id=23353599">
  <h1 class="title" title="-----">-----</h1>
</a>

2 anchor-tag when getting imageTitle

<h1 class="title">
  <a class="_icon sprites-logo" href="http://www.pixiv.net/mypage.php">pixiv</a>
</h1>,
<h1 class="title">Cos-Nurse</h1>

e5: for python3.4 traceback.format_exc

@rachmadaniHaryono
Copy link
Contributor

rachmadaniHaryono commented Jul 23, 2018

i'm trying a new strategy with this branch https://github.com/rachmadaniHaryono/PixivUtil2/tree/feature/py3

rules:

  • do least minimum changes
  • use six lib as much as possible
  • use mypy when required (not happen, but may be used later)
    notes:
  • on debugging i met line where module have to be wrapped with socks module. i don't know how to test it for python3, so i will skip it for urllib and urllib2. httplib have direct alias in six (six.moves.http_client)
  • it maybe better to isolate python3 function/method and let it raise error when debugging active (wip). see case below

so when this happen

Trying to log in with saved cookie
Error at doLogin(): (<class 'AttributeError'>, AttributeError("'Response' object has no attribute 'read'",), <traceback object at 0x7f863a363048>)
Cannot Login!
press enter to exit.

do this:

  • set if-else branch where the function is called
  • copy doLogin function to doLoginOnPython3
  • (optional) call the original function
  • on error check if env vars for debug is enabled (something like PIXIVUTIL2_DEBUG).
    • if it is, raise actual error and print trace back
    • if not, use default error handler

e1: add #387 for easier migration

also working on mypy test.
notes:

  • that mypy can only be installed on >python3.4 so no direct mypy travis test on python2.7
  • mypy test most of the time raise module related error. ignore-missing-imports flag will skip most of them
  • typing package is required on python2.7, based on this https://mypy.readthedocs.io/en/latest/python2.html.
  • all no annotation error fixed by adding Any-type. no attribute error is fixed by skipping the line (add type: ignore comment on the line)

@Qwerty-Space
Copy link

To further this, @Nandaka, python 2 EOL is at the end of this year. a python 3 branch should probably be the top priority.

fake-name added a commit to fake-name/PixivUtil2 that referenced this issue Sep 21, 2019
fake-name added a commit to fake-name/PixivUtil2 that referenced this issue Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants