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

Modernize Python 2 code to prepare for Python 3 #60

Closed
wants to merge 1 commit into from

Conversation

cclauss
Copy link

@cclauss cclauss commented Jul 12, 2019

@garethdmm @zeapo @bmoscon @asmodehn Please try out this executable on Python 2.7 and see how it works. All tests pass on both Python 2.7 and Python 3.6 with 172 files modified. Python 3.7 tests must be run in allow_failures mode until rkern/line_profiler#153 (or similar) is merged and pushed to pypi.

Four undefined names still exist on both Python 2 and Python 3...
$ flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

@cclauss cclauss changed the title Let’s run the flake8 test on the Python 3 branch Modernize Python 2 code to prepair for Python 3 Jul 12, 2019
@cclauss cclauss force-pushed the patch-3 branch 2 times, most recently from a7f611d to ca6137a Compare July 13, 2019 00:54
@cclauss cclauss changed the title Modernize Python 2 code to prepair for Python 3 Modernize Python 2 code to prepare for Python 3 Jul 13, 2019
@cclauss
Copy link
Author

cclauss commented Jul 13, 2019

Fixed all the unicode() issues so there is now just one Python 3 FAIL in gryphon/tests/logic/libraries/arbitrage_test.py. Any ideas?

@cclauss cclauss force-pushed the patch-3 branch 4 times, most recently from f28696f to 3eb5817 Compare July 16, 2019 13:27
@cclauss
Copy link
Author

cclauss commented Jul 16, 2019

Updated the message above because All tests pass on both Python 2.7 and Python 3.6.

@asmodehn
Copy link
Contributor

I tested this branch with 3.7 only. Here are some quick notes :

25) ERROR: Failure: ImportError (Building module gryphon.lib.models.exchange failed: ['ImportError: Building module gryphon.lib.models.order failed: [\'ImportError: Building module gryphon.lib.models.trade failed: ["sqlalchemy.exc.InvalidRequestError: Table \\\'trade\\\' is already defined for this MetaData instance.  Specify \\\'extend_existing=True\\\' to redefine options and columns on an existing Table object.\\\\n"]\\n\']\n'])

Not sure what to do about it...

Hope this helps. Don't hesitate to ping me on slack to talk about this, I d like to help to get python3 working asap :-)

@asmodehn
Copy link
Contributor

On Python 2.7, I tried to merge these changes with mine and run gryphon-runtests but I ended up with a lot of ImportError, so I m not sure what I am doing wrong here, probably my merge broke something somewhere... I ll report if I eventually get something running.

Copy link
Contributor

@asmodehn asmodehn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for me !

@@ -2,6 +2,8 @@

from sqlalchemy import func

#from gryphon.lib.gryphonfury.revenue import (get_start_and_end_position,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about just remove this ?

@asmodehn
Copy link
Contributor

Doubting my previous merges, I did a vanilla clone&runtests of that branch (on py2.7) and everything went fine.
Also I reviewed all changes and everything looks like the usual code for supporting py2&py3 together.

So I am all in support to merge this asap so I can fix my code and PRs to work with it ;-)

@cclauss
Copy link
Author

cclauss commented Jul 24, 2019

As @bmoscon rightly pointed out, passing the tests in one thing but using the software is another. Is it possible for you to try out this software as a user would use it on Python 2 and 3 and let me know if it works as expected?

@asmodehn
Copy link
Contributor

I am using a modified version of gryphon, and a blind merge didn't work, so I ll need to port my changes one by one and investigate what I did that break things.

Also my use of gryphon is very limited, so I am not sure it would even be that useful.
Anyway I ll do another attempt sometime this week, probably another setup from scratch.

@asmodehn
Copy link
Contributor

Attempting to run gryphon-dashboards with this branch, I stumbled upon issues I had with gryphon previously that have not been addressed yet. So this is "working as expected" for me :

  File "/home/alexv/.virtualenvs/gryphon2up/local/lib/python2.7/site-packages/tornadotoad/api.py", line 141, in _send
    if tornado.ioloop.IOLoop.initialized():
AttributeError: type object 'IOLoop' has no attribute 'initialized'

#23

  File "/opt/Projects/cclaus_gryphon/gryphon/lib/exchange/exchange_factory.py", line 145, in make_exchange_datas_from_keys
    assert len(exchange_datas) == len(pair_names)
AssertionError

#25

And some I don't remember reporting, probably because I suspect they happen sparsely and may be related to my specific setup :

  File "/home/alexv/.virtualenvs/gryphon2up/local/lib/python2.7/site-packages/tornado/template.py", line 346, in generate
    return execute()
  File "templates/strategy_html.generated.py", line 107, in _tt_execute
    _tt_tmp = '%s %.2f' % (args['open_pl'].currency, args['open_pl'].amount)  # templates/strategy.html:51 (via templates/base.html:207)
AttributeError: 'NoneType' object has no attribute 'currency'
/home/alexv/.virtualenvs/gryphon2up/local/lib/python2.7/site-packages/sqlalchemy/engine/default.py:536: Warning: Truncated incorrect datetime value: '2019-07-25 00:00:00+00:00'
  cursor.execute(statement, parameters)

Running the simple_market_making builtin strategy also works as it would on the current master.

@asmodehn
Copy link
Contributor

asmodehn commented Jul 25, 2019

I have been trying to run tests with this branch, and I get some strange failure for imports, if I happen to run tests without using without using gryphon-runtests

So I would actually put :

from __future__ import absolute_import

at the beginning of every module. Better safe than sorry.

EDIT: To get what I mean, just run for example :

(gryphon2)alexv@pop-os:/opt/Projects/gryphon$ nosetests gryphon.tests

This was referenced Jul 29, 2019
@cclauss
Copy link
Author

cclauss commented Aug 13, 2019

@garethdmm Should I closed this PR?

@asmodehn
Copy link
Contributor

Do we have anything better for supporting python3 ?

@zahnz
Copy link

zahnz commented Mar 30, 2020

What's the status on this and the other Python3 WIP issue that bmoscon started? Are there any points that need to be resolved between the two PRs? Can anyone summarize what's currently needed for this to move forward? I'm willing to contribute.

@pjz
Copy link

pjz commented Aug 19, 2020

I decided to try and make this work, so I did this:

  • upgrade line-profiler to the latest version (3.0.2)
  • upgrade sure to the latest version (1.4.11)

And now I get:

ImportError: Building module gryphon.execution.strategies.builtin.multiexchange_linear failed: ['ImportError: Building module gryphon.execution.strategies.base failed: [\'ImportError: Building module gryphon.lib.models.exchange failed: [\\\'ImportError: Building module gryphon.lib.models.order failed: [\\\\\\\'ImportError: Building module gryphon.lib.models.trade failed: ["sqlalchemy.exc.InvalidRequestError: Table \\\\\\\\\\\\\\\'trade\\\\\\\\\\\\\\\' is already defined for this MetaData instance.  Specify \\\\\\\\\\\\\\\'extend_existing=True\\\\\\\\\\\\\\\' to redefine options and columns on an existing Table object.\\\\\\\\\\\\\\\\n"]\\\\\\\\n\\\\\\\']\\\\n\\\']\\n\']\n']

Which looks like some kind of sqlalchemy error. I can fix it by adding

 __table_args__ = {'extend_existing': True}

under the __tablename__ = 'trade' line in gryphon/lib/models/trade.pyx , but after that it complains about the mutual imports of Trade and Order :

   ImportError: Building module gryphon.execution.strategies.builtin.multiexchange_linear failed: ['ImportError: Building module gryphon.execution.strategies.base failed: [\'ImportError: Building module gryphon.lib.models.exchange failed: [\\\'ImportError: Building module gryphon.lib.models.order failed: ["ImportError: Building module gryphon.lib.models.trade failed: [\\\\\\\'ImportError: cannot import name Order\\\\\\\\\\\\\\\\n\\\\\\\']\\\\\\\\n"]\\\\n\\\']\\n\']\n']

...which takes more sqlalchemy-fu to fix than I know. I guess I'll go level it up, but just know that this isn't completely moribund.

@cclauss cclauss closed this Jun 17, 2021
@cclauss cclauss deleted the patch-3 branch June 17, 2021 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants