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

Flake & Make #273

Merged
merged 28 commits into from
Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
572c6a8
initial stab - without applying flake
Apr 12, 2022
b5301b8
bring in examples
Apr 12, 2022
a4d5314
pull optimizer.* up a level
Apr 12, 2022
f4c45d0
more flakes
Apr 12, 2022
90b70c8
remove future changes
Apr 12, 2022
1d9903e
add make instructions
Apr 12, 2022
ce41ea6
Merge remote-tracking branch 'origin/master' into make-and-flake
Apr 12, 2022
a892766
apply lint to tests/ too
Apr 12, 2022
6e4df41
EOF
Apr 12, 2022
73d1d0c
narrow the ignores per CR suggestion
Apr 13, 2022
f0e9b76
`make docs` idempotent per CR suggestion
Apr 13, 2022
00852fc
Remove redundant test imports
michaeldiamant Apr 14, 2022
405a6fe
per CR suggestions
Apr 14, 2022
c709869
per CR suggestions
Apr 14, 2022
06ac7f5
Merge branch 'make-and-flake_remove_redundant_imports' of https://git…
Apr 14, 2022
c4ea3fa
Remove redundant test imports (#277)
michaeldiamant Apr 14, 2022
30a62bc
no more blanket import * ignores - ignore in library code, but use pt…
Apr 14, 2022
74ca33e
exclude module_test.py from import * disallowance
Apr 14, 2022
69dcbea
mardkownlint for the README.md
Apr 14, 2022
34a1464
fix the flake8 guidance
Apr 14, 2022
084e2ce
mypy pyteal only in readme
Apr 14, 2022
ebb13f1
per CR catch
Apr 14, 2022
bbb6b8a
better formatting
Apr 14, 2022
72a78a0
Merge branch 'master', remote-tracking branch 'origin' into make-and-…
Apr 14, 2022
008fb5a
revert per CR comments
Apr 19, 2022
31a4612
revert per CR comments
Apr 19, 2022
cd65173
move ignore to .flake8
Apr 19, 2022
1f1e246
Merge remote-tracking branch 'origin/master' into make-and-flake
Apr 19, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
[flake8]
Copy link
Contributor

Choose a reason for hiding this comment

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

@tzaffi For https://www.flake8rules.com/rules/F405.html and https://www.flake8rules.com/rules/F403.html, I'm imagining we'd prefer to avoid the violation in source code. Does it work for you to limit ignoring F403 + F405 to *_test.py and examples/?

From the previous attempt, there's an example ignoring *_test.py: https://github.com/algorand/pyteal/pull/213/files#diff-6951dbb399883798a226c1fb496fdb4183b1ab48865e75eddecf6ceb6cf46442R11.

Running locally, I see there's a F403 violation in source code. I didn't look closely enough to determine how easily it can be avoided vs adding inline ignore. I can take a deeper look if you'd like - let me know.

~/dev/pyteal make-and-flake *1 !1 ?6 ────────────────────────────────────────────────────────────────────────── ✘ 2|1 5s  pyteal 09:53:33 AM
❯ make flake8 | grep -v -E 'test|examples'
pyteal/__init__.py:1:1: F403 'from .ast import *' used; unable to detect undefined names
pyteal/__init__.py:3:1: F403 'from .ir import *' used; unable to detect undefined names
make: *** [flake8] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll research this some more and update here.

Copy link
Contributor Author

@tzaffi tzaffi Apr 13, 2022

Choose a reason for hiding this comment

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

When I first joined I didn't like our profligate use of * imports in pyteal. But the more I use pyteal, the more this convention makes sense to me. We have so many constructs that are typically used in pyteal, that a standard pyteal program would have 10 or more objects being imported.

An alternate approach in such situations (where a library with many constructs is used) is to abbreviate the imported module and then always refer to it. For example:

import pandas as pd

...
df = pd.DataFrame(...)

we could follow a similar convention and start enforcing it here with usages such as below (notice, we are already implicitly recommending this approach with abi):

import pyteal as pt
from pyteal import abi

toSum = abi.DynamicArray(abi.Uint64TypeSpec())
toMultiply = abi.DynamicArray(abi.Uint64TypeSpec())
results = abi.Tuple(abi.Uint64TypeSpec(), abi.Uint64TypeSpec())
program = pt.Seq(
    toSum.decode(pt.Txn.application_args[0]),
    toMultiply.decode(pt.Txn.application_args[1]),
    sumAndMultiply(toSum, toMultiply).store_into(results)
)

Copy link
Contributor

Choose a reason for hiding this comment

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

A few thoughts from my side:

  • If folks are on-board, I am on-board to use the pandas as pd approach. I think it'd help disambiguate PyTeal constructs that match Python standard library (e.g. Array).
  • If folks prefer to keep as is, I'd advocate to add the ignore only for tests and examples. I'm thinking in source code we can be more perspective. Though I'll ultimately follow the group's recommendation here.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's 3 separate issues:

  • Use of star imports in our runtime code: this is highly useful because we have a few modules that import and export all of a submodule. It would be too time consuming and error prone to not use a star import here. My opinion is that adding inline exceptions for these cases is good enough.
  • Use of star imports in example: from the beginning we've encouraged this because PyTeal programs can use a lot of different AST elements, and because our syntax is already clunky enough without added prefixed in my opinion. I'm in favor of keeping it as-is since our examples should look natural and not be intimidating.
  • Use of star imports in our tests: this matters least to me, so if you'd like to adopt import pyteal as pt or similar, I wouldn't mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go with the modifying the tests files only and adding flake8 exceptions for other files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe... that resulted in 6000+ changes

Copy link
Contributor

Choose a reason for hiding this comment

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

@tzaffi Thanks for the changes here - I think it's reasonable starting point and we can iterate over time. Resolving.

ignore =
E203,
E241,
W291,
E302,
E501,
W503,
E741,

per-file-ignores =
pyteal/compiler/optimizer/__init__.py: F401
examples/application/asset.py: F403, F405
examples/application/security_token.py: F403, F405
examples/application/vote.py: F403, F405
examples/signature/atomic_swap.py: F403, F405
examples/signature/basic.py: F403, F405
examples/signature/dutch_auction.py: F403, F405
examples/signature/periodic_payment_deploy.py: F403, F405
examples/signature/recurring_swap.py: F403, F405
examples/signature/split.py: F403, F405
examples/signature/periodic_payment.py: F403, F405
examples/signature/recurring_swap_deploy.py: F403, F405
pyteal/__init__.py: F401, F403
pyteal/ir/ops.py: E221
tests/module_test.py: F401, F403
34 changes: 12 additions & 22 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ on:
- v**
branches:
- master

jobs:
build-test:
runs-on: ubuntu-20.04
container: python:${{ matrix.python }}-slim
container: python:${{ matrix.python }}
strategy:
matrix:
python: ['3.10']
Expand All @@ -19,16 +20,11 @@ jobs:
uses: actions/checkout@v2
with:
fetch-depth: 0
- name: Install pip dependencies
run: |
pip install -r requirements.txt
pip install -e .
- name: Install python dependencies
run: make setup-development
- name: Build and Test
run: |
python scripts/generate_init.py --check
black --check .
mypy pyteal
pytest
run: make build-and-test

build-docset:
runs-on: ubuntu-20.04
container: python:3.10 # Needs `make`, can't be slim
Expand All @@ -37,22 +33,16 @@ jobs:
uses: actions/checkout@v2
with:
fetch-depth: 0
- name: Install pip dependencies
run: |
pip install -r requirements.txt
pip install -r docs/requirements.txt
pip install doc2dash
- name: Install python dependencies
run: make setup-docs
- name: Make docs
run: |
cd docs
make html
doc2dash --name pyteal --index-page index.html --online-redirect-url https://pyteal.readthedocs.io/en/ _build/html
tar -czvf pyteal.docset.tar.gz pyteal.docset
run: make bundle-docs
- name: Archive docset
uses: actions/upload-artifact@v2
with:
name: pyteal.docset
path: docs/pyteal.docset.tar.gz

upload-to-pypi:
runs-on: ubuntu-20.04
container: python:3.10
Expand All @@ -64,9 +54,9 @@ jobs:
with:
fetch-depth: 0
- name: Install dependencies
run: pip install wheel
run: make setup-wheel
- name: Build package
run: python setup.py sdist bdist_wheel
run: make bdist-wheel
- name: Release
uses: pypa/gh-action-pypi-publish@release/v1
with:
Expand Down
10 changes: 6 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
.DS_Store

# Byte-compiled / optimized / DLL files
__pycache__/
*.py[cod]
Expand Down Expand Up @@ -54,8 +52,8 @@ coverage.xml
.pytest_cache/

# Tests generating TEAL output to compared against an expected example.
tests/teal/*.teal
!tests/teal/*_expected.teal
tests/**/*.teal
!tests/**/*_expected.teal

# Translations
*.mo
Expand All @@ -76,6 +74,7 @@ instance/

# Sphinx documentation
docs/_build/
pyteal.docset*

# PyBuilder
target/
Expand Down Expand Up @@ -133,3 +132,6 @@ dmypy.json
# IDE
.idea
.vscode

# mac OS
.DS_Store
49 changes: 49 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
setup-development:
pip install -e.[development]

setup-docs: setup-development
pip install -r docs/requirements.txt
pip install doc2dash

setup-wheel:
pip install wheel

bdist-wheel:
python setup.py sdist bdist_wheel

bundle-docs-clean:
rm -rf docs/pyteal.docset

bundle-docs: bundle-docs-clean
cd docs && \
make html && \
doc2dash --name pyteal --index-page index.html --online-redirect-url https://pyteal.readthedocs.io/en/ _build/html && \
tar -czvf pyteal.docset.tar.gz pyteal.docset

generate-init:
python -m scripts.generate_init

check-generate-init:
python -m scripts.generate_init --check

ALLPY = docs examples pyteal scripts tests *.py
black:
black --check $(ALLPY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why $(ALLPY) and not .?

Copy link
Contributor Author

@tzaffi tzaffi Apr 14, 2022

Choose a reason for hiding this comment

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

It was going into my virutal env directory and failing because of that, so I wanted to explicitly define what is being checked. It's also handy to use this for the other linters (flake8 and eventually mypy). So I would prefer to keep this way.

Another approach is to specifically exclude certain folders with --exclude, but then everyone would have to configure that specifically for their local excludes.

But admittedly, the downside is that if you add another top-level directory, we wouldn't be formatting it by default.

I don't feel strongly about this. LMK


flake8:
flake8 $(ALLPY)

MYPY = pyteal scripts
mypy:
mypy $(MYPY)

lint: black flake8 mypy

test-unit:
pytest

build-and-test: check-generate-init lint test-unit

# Extras:
coverage:
pytest --cov-report html --cov=pyteal
47 changes: 29 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
![PyTeal logo](https://github.com/algorand/pyteal/blob/master/docs/pyteal.png?raw=true)
<!-- markdownlint-disable-file MD041 -->

![PyTeal logo](https://github.com/algorand/pyteal/blob/master/docs/pyteal.png?raw=true)

# PyTeal: Algorand Smart Contracts in Python

Expand All @@ -8,59 +9,69 @@
[![Documentation Status](https://readthedocs.org/projects/pyteal/badge/?version=latest)](https://pyteal.readthedocs.io/en/latest/?badge=latest)
[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black)

PyTeal is a Python language binding for [Algorand Smart Contracts (ASC1s)](https://developer.algorand.org/docs/features/asc1/).
PyTeal is a Python language binding for [Algorand Smart Contracts (ASC1s)](https://developer.algorand.org/docs/features/asc1/).

Algorand Smart Contracts are implemented using a new language that is stack-based,
called [Transaction Execution Approval Language (TEAL)](https://developer.algorand.org/docs/features/asc1/teal/).
Algorand Smart Contracts are implemented using a new language that is stack-based,
called [Transaction Execution Approval Language (TEAL)](https://developer.algorand.org/docs/features/asc1/teal/).

However, TEAL is essentially an assembly language. With PyTeal, developers can express smart contract logic purely using Python.
However, TEAL is essentially an assembly language. With PyTeal, developers can express smart contract logic purely using Python.
PyTeal provides high level, functional programming style abstractions over TEAL and does type checking at construction time.

### Install
## Install

PyTeal requires Python version >= 3.10.

To manage multiple Python versions use tooling like [pyenv](https://github.com/pyenv/pyenv).

#### Recommended: Install from PyPi
### Recommended: Install from PyPi

Install the latest official release from PyPi:

* `pip install pyteal`

#### Install Latest Commit
### Install Latest Commit

If needed, it's possible to install directly from the latest commit on master to use unreleased features:

> **WARNING:** Unreleased code is experimental and may not be backwards compatible or function properly. Use extreme caution when installing PyTeal this way.

* `pip install git+https://github.com/algorand/pyteal`

### Documentation
## Documentation

* [PyTeal Docs](https://pyteal.readthedocs.io/)
* `docs/` ([README](docs/README.md)) contains raw docs.

### Development Setup
## Development Setup

Setup venv (one time):
* `python3 -m venv venv`

* `python3 -m venv venv`

Active venv:
* `. venv/bin/activate` (if your shell is bash/zsh)
* `. venv/bin/activate.fish` (if your shell is fish)

Pip install PyTeal in editable state
* `pip install -e .`
* `. venv/bin/activate` (if your shell is bash/zsh)
* `. venv/bin/activate.fish` (if your shell is fish)

Pip install PyTeal in editable state with dependencies:

* `make setup-development`
* OR if you don't have `make` installed:
* `pip install -e.[development]`
* Note, that if you're using `zsh` you'll need to escape the brackets: `pip install -e.\[development\]`

Install dependencies:
* `pip install -r requirements.txt`

Type checking using mypy:

* `mypy pyteal`

Run tests:

* `pytest`

Format code:

* `black .`

Lint using flake8:

* `flake8 docs examples pyteal scripts tests *.py`
1 change: 0 additions & 1 deletion examples/application/vote_deploy.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# based off https://github.com/algorand/docs/blob/cdf11d48a4b1168752e6ccaf77c8b9e8e599713a/examples/smart_contracts/v2/python/stateful_smart_contracts.py

import base64
import datetime

from algosdk.future import transaction
from algosdk import account, mnemonic
Expand Down
10 changes: 7 additions & 3 deletions examples/signature/periodic_payment_deploy.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
#!/usr/bin/env python3

from pyteal import *
import uuid, params, base64
import base64
import params
import uuid

from algosdk import algod, transaction, account, mnemonic
from periodic_payment import periodic_payment, tmpl_amt
from periodic_payment import periodic_payment

from pyteal import *

# --------- compile & send transaction using Goal and Python SDK ----------

Expand Down
16 changes: 12 additions & 4 deletions examples/signature/recurring_swap_deploy.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
#!/usr/bin/env python3

from pyteal import *
import base64
from nacl import encoding, hash
from recurring_swap import recurring_swap, tmpl_provider
from algosdk import algod, account
import params
import re
import time
import uuid

from algosdk import algod
from algosdk.future import transaction
import uuid, params, re, base64, time

from pyteal import *

from recurring_swap import recurring_swap


# ------- generate provider's account -----------------------------------------------
key_fn = str(uuid.uuid4()) + ".key"
Expand Down
5 changes: 0 additions & 5 deletions pyteal/ast/acct.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
from typing import TYPE_CHECKING

from ..types import TealType, require_type
from ..ir import Op
from .expr import Expr
from .maybe import MaybeValue

if TYPE_CHECKING:
from ..compiler import CompileOptions


class AccountParam:
@classmethod
Expand Down
Loading