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

Enhancement: Type Friendly Exports #435

Merged
merged 25 commits into from Jan 18, 2023
Merged

Enhancement: Type Friendly Exports #435

merged 25 commits into from Jan 18, 2023

Conversation

tzaffi
Copy link
Contributor

@tzaffi tzaffi commented Jan 15, 2023

Introduce PyTeal's fix for VsCode type exports issue

This approach was introduced into PyTeal by @barnjamin. I'm just bringing it into this repo.

Former VsCode Pylance error when importing py-algorand-sdk:

image

Testing

  • There is no specific test in C.I. to prove that it works. But it "works on my machine"
  • Also introducing python 3.11 into the C.I. workflow

@tzaffi tzaffi changed the title Type Friendly Exports Enhancement: Type Friendly Exports Jan 16, 2023
@tzaffi tzaffi marked this pull request as ready for review January 16, 2023 16:08
Comment on lines 27 to 29
- run: make lint
- run: make check-setup
- run: make pytest-unit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bringing these commands into the Makefile

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

Explicitly defining __all__ seems like a good practice. Strangely, I don't notice any PyLance errors on my VSCode.

Makefile Outdated Show resolved Hide resolved
algosdk/__init__.pyi Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@tzaffi
Copy link
Contributor Author

tzaffi commented Jan 17, 2023

@algochoi - please have a look at my latest commit 6e70ed0

I believe that most of your points have been addressed.

Copy link
Contributor

@algochoi algochoi left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants