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

feat: getattr and iter dunders #42

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

BobTheBuidler
Copy link

What I did

added getattr and iter dundermethods to the token manager

fixes: #

How I did it

n/a

How to verify it

try to iter and getattr

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

ape_tokens/managers.py Outdated Show resolved Hide resolved
def __len__(self) -> int:
return len(list(self._manager.get_tokens(chain_id=self.provider.chain_id)))

def __iter__(self) -> Iterator[ContractInstance]:
Copy link
Member

Choose a reason for hiding this comment

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

I tried running:

[x for x in tokens]

but got this:

KeyError: "Symbol 'AZRX' is not a known token symbol"

In [2]: [x for x in tokens]

kinda weird. I do have multiple token lists installed though.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if something like:

 def __iter__(self) -> Iterator[ContractInstance]:
        yielded = set()
        for token in self._manager.get_tokens(chain_id=self.provider.chain_id):
            if token.symbol in yielded:
                # Handle multiple lists.
                continue

            try:
                yield self[token.symbol]
            except KeyError:
                # Perhaps in another list?
                continue

            yielded.add(token.symbol)

would work for this situation.
I tried it again, except changed it to:

for x in tokens:
   ...:     print(x)

I see the first like hundred get printed out super fast, and then it suddenly is very slow at outputting them.
hmmmmm

Copy link
Author

Choose a reason for hiding this comment

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

is it possible the slowness is from downloading the last of the tokenlists? Looking at this code again I'm not sure where else a slowdown might come from.

Copy link
Author

Choose a reason for hiding this comment

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

I think this should be fixed now with my latest commit.


except ValueError as err:
def __iter__(self) -> Iterator[ContractInstance]:
for token in self._manager.get_tokens(chain_id=self.provider.chain_id):
Copy link
Member

@antazoey antazoey May 23, 2024

Choose a reason for hiding this comment

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

I am getting the KeyError here when doing [x for x in token]

In [1]: [x for x in tokens]
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[1], line 1
----> 1 [x for x in tokens]

File ~/PycharmProjects/ape-tokens/ape_tokens/managers.py:124, in TokenManager.__iter__(self)
    122 def __iter__(self) -> Iterator[ContractInstance]:
    123     for token in self._manager.get_tokens(chain_id=self.provider.chain_id):
--> 124         yield self[token.symbol]

File ~/PycharmProjects/ape-tokens/ape_tokens/managers.py:137, in TokenManager.__getitem__(self, symbol)
    134         continue
    136 if token_info is None:
--> 137     raise KeyError(f"Symbol '{symbol}' is not a known token symbol")
    139 checksummed_address = to_checksum_address(token_info.address)
    140 try:

KeyError: "Symbol 'GETH' is not a known token symbol"

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.

2 participants