dydx-v3-python icon indicating copy to clipboard operation
dydx-v3-python copied to clipboard

Sir/python_3114

Open sirEven opened this issue 2 years ago • 11 comments

This PR solves compatibility issues when using dydx-v3-python with python 3.11.4 The issues stem from conflicting imports by the parsimonious package, which is used by eth-abi package, which in turn is used by both eth-account and web3 packages.

The following github issues seem related to this PR:

  • https://github.com/dydxprotocol/dydx-v3-python/issues/209
  • https://github.com/dydxprotocol/dydx-v3-python/issues/187 As well as this older PR:
    • Which is similar albeit harshly lifting upper version constraints completely, which will lead to more compatibility issues in the future
    • https://github.com/dydxprotocol/dydx-v3-python/pull/202/files

What this PR does:

  • move upper version constraint for the following packages to current versions in both requirements.txt and requirements-test.txt:

    • eth-account
    • web3
  • modify to include new package versions and python 3.11.4 here:

    • tox.ini
    • setup.py
  • change source code to rename these function calls (see corresponding resources in parentheses):

    • solidityKeccak -> solidity_keccak (https://web3py.readthedocs.io/en/stable/releases.html#id54)
    • signTypedData -> sign_typed_data (https://web3py.readthedocs.io/en/stable/releases.html#id124)
    • defaultAccount -> default_account (https://web3py.readthedocs.io/en/stable/releases.html#id129)
    • recoverHash -> _recover_hash (https://web3py.readthedocs.io/en/v5/web3.eth.account.html)

sirEven avatar Sep 03 '23 14:09 sirEven

I would like to install the client for v3 alongside v4 for Python 3.11.x via poetry.

$ poetry add git+https://github.com/sirEven/dydx-v3-python.git#sir/python_3114
$ poetry add git+https://github.com/dydxprotocol/v4-clients.git@main#subdirectory=v4-client-py

Unfortunately, this raises a conflict with the sympy dependency. How about changing the fixed version for sympy to an upper version constraint in preparation for v4 i.e. ^1.12.0?

christophlins avatar Oct 03 '23 15:10 christophlins

Hello guys, wen this PR will be merged ?

Otnicka1 avatar Oct 24 '23 20:10 Otnicka1

I would like to install the client for v3 alongside v4 for Python 3.11.x via poetry.

$ poetry add git+https://github.com/sirEven/dydx-v3-python.git#sir/python_3114
$ poetry add git+https://github.com/dydxprotocol/v4-clients.git@main#subdirectory=v4-client-py

Unfortunately, this raises a conflict with the sympy dependency. How about changing the fixed version for sympy to an upper version constraint in preparation for v4 i.e. ^1.12.0?

Hey there, i just tried to up the constraint to sympy<=1.12.0 - which would be fine as far as i can tell! How ever there is now one test failing (independent of your requested requirements change): Apparently somehow there seems to be a user called "foo" on the ganache test network all of a sudden, such that the test "def test_check_if_username_exists(self):" fails because it returns true instead of false - while i think this is hilarious, i don't quite understand how this has come to be... I posted a question in the dev channel on discord to resolve this issue. My quick fix currently is to check for "fooo" instead in this test to make it green again. However i decided to wait a little while on some feedback before proceeding once i can push, i let you know so you can use the new version with the desired constraints! Cheers, S.

Edit: Never mind my hesitation, i just upped the constraint to your needs and fixed the test along the way by changing the user name to smth else. I hope your undertaking works now! S.

sirEven avatar Oct 24 '23 21:10 sirEven

Edit: Never mind my hesitation, i just upped the constraint to your needs and fixed the test along the way by changing the user name to smth else. I hope your undertaking works now! S.

Many thanks! I am just wondering why you wouldn't change it to sympy>=1.12.0. This reflects the dependency constraint in v4.

christophlins avatar Oct 25 '23 17:10 christophlins

Edit: Never mind my hesitation, i just upped the constraint to your needs and fixed the test along the way by changing the user name to smth else. I hope your undertaking works now! S.

Many thanks! I am just wondering why you wouldn't change it to sympy>=1.12.0. This reflects the dependency constraint in v4.

Hey man, so sorry, i misunderstood you! I Hope it works now for you as i just pushed them to >= 1.12.0 :) Cheers, S.

sirEven avatar Oct 25 '23 18:10 sirEven

@christophlins Do you have any plans to make a repo source release? I have prepared a conda recipe, but since there is not a repo release, I used the pypi package with heavy modifications (along the lines of @sirEven PR). This may not be accepted (recipe maintainers are not allowed to make changes) Incidentally, is there someone interested in being a co-maintainer for the conda recipe? Recipe (in PR state): https://github.com/conda-forge/staged-recipes/pull/23806

MementoRC avatar Oct 25 '23 20:10 MementoRC

@christophlins Do you have any plans to make a repo source release?

No. For the time being, I rely on this PR until it gets merged and eventually releases by the dydx folks.

christophlins avatar Oct 25 '23 22:10 christophlins

@sirEven

Unfortunately, more dependencies mismatches popped up... Comparing v3 with v4 requirements, I think the following changes are necessary:

cytoolz>=0.12.1
dateparser>=1.0.0
ecdsa>=0.18.0
eth-account>=0.9.0
mpmath>=1.3.0
requests>=2.31.0
six>=1.16.0
web3>=6.5.0

Would you mind giving it a spin? If it doesn't work straight away, it might not be worth the effort...

christophlins avatar Oct 27 '23 12:10 christophlins

@sirEven

Unfortunately, more dependencies mismatches popped up... Comparing v3 with v4 requirements, I think the following changes are necessary:

cytoolz>=0.12.1
dateparser>=1.0.0
ecdsa>=0.18.0
eth-account>=0.9.0
mpmath>=1.3.0
requests>=2.31.0
six>=1.16.0
web3>=6.5.0

Would you mind giving it a spin? If it doesn't work straight away, it might not be worth the effort...

hey @christophlins I just upped the dependencies as you suggested, let me know if that helps! Cheers, S.

sirEven avatar Oct 27 '23 15:10 sirEven

hey @christophlins I just upped the dependencies as you suggested, let me know if that helps! Cheers, S.

@sirEven It works like a charm. Many thanks for the quick turnaround. Highly appreciated!

christophlins avatar Oct 27 '23 15:10 christophlins

@christophlins Cool, I mistakenly thought you were a maintainer

No. For the time being, I rely on this PR until it gets merged and eventually releases by the dydx folks.

MementoRC avatar Nov 01 '23 23:11 MementoRC