Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Feature/python/new_demos #1726

Closed

Conversation

michaelhly
Copy link
Contributor

@michaelhly michaelhly commented Mar 24, 2019

Description

New demos to demonstrate usage of python packages.

Testing instructions

./setup.py/launch_kit
./setup.py/test

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@michaelhly michaelhly requested a review from feuGeneA as a code owner March 24, 2019 04:58
@michaelhly michaelhly changed the title [WIP] Feature/python/new demos [WIP] Feature/python/new_demos Mar 24, 2019
@michaelhly
Copy link
Contributor Author

Blocked until #1730 is merged.

Copy link
Contributor

@feuGeneA feuGeneA left a comment

Choose a reason for hiding this comment

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

I must admit I'm a little confused about this one. The content is looking great, but I'm not sure this is the right place for it. I would think that a lot of these demos would be better placed in the docstrings of the packages/modules they're demonstrating. I added more specific comments for each one.

"""Demonstration of creating and validating orders."""


def create_order():
Copy link
Contributor

Choose a reason for hiding this comment

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

I would think this demo would be better placed in the zero_ex.order_utils module docstring.

def validate_order():
"""Validate a 0x order.

>>> from zero_ex.json_schemas import assert_valid
Copy link
Contributor

Choose a reason for hiding this comment

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

I would think this demo would be better placed in the zero_ex.json_schemas module docstring.

@@ -0,0 +1,137 @@
"""Demonstration of using the python sra_client."""
Copy link
Contributor

Choose a reason for hiding this comment

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

I would think this demo would be better placed in the zero_ex.sra_client module docstring.

... network_id=42, signed_order_schema=example_signed_order)
>>> response[1]
200
""" # noqa: E501
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" # noqa: E501
""" # noqa: E501 (line too long)

'taker_asset_amount': '500000000000000000000',
'taker_asset_data': '0xf47261b00000000000000000000000002002d3812f58e35f0ea1ffbf80a75a38c32175fa',
'taker_fee': '0'}}]}
""" # noqa: E501
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" # noqa: E501
""" # noqa: E501 (line too long)

'taker_asset_data': '0xf47261b00000000000000000000000002002d3812f58e35f0ea1ffbf80a75a38c32175fa',
'taker_fee': '0'}},
""" # noqa: 501
# NOTE: sra_client not deserialzing order from server properly, need fix!
Copy link
Contributor

Choose a reason for hiding this comment

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

Please raise a GitHub Issue to describe this problem.

'maxAmount': '115792089237316195423570985008687907853269984665640564039457584007913129639936',
'minAmount': '0',
'precision': 18}}]}
""" # noqa: E501
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" # noqa: E501
""" # noqa: E501 (line too long)

'taker_asset_data': '0xf47261b00000000000000000000000002002d3812f58e35f0ea1ffbf80a75a38c32175fa',
'taker_fee': '0'}}]},
'bids': {'records': []}}
""" # noqa: E501
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" # noqa: E501
""" # noqa: E501 (line too long)

To interact with a 0x-relayer, you need the HTTP endpoint of the relayer you'd like to
connect to (i.e. https://api.radarrelay.com/0x/v2).

For local testing one use the `0x-launch-kit
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For local testing one use the `0x-launch-kit
For local testing one can use the `0x-launch-kit

@@ -0,0 +1,68 @@
.. source for the sphinx-generated build/docs/web/index.html
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this content belongs in the sra_client module docstring, and the sphinx generation should happen there too.

@michaelhly michaelhly force-pushed the feature/python/new-demos branch from e2dab1a to 374ef75 Compare March 25, 2019 18:32
@michaelhly michaelhly force-pushed the feature/python/new-demos branch from a24b68d to d854e31 Compare March 25, 2019 19:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants