Skip to content

fix: support tokenlists v0.2 - #72

Open
fubuloubu wants to merge 5 commits into
mainfrom
fix/support-tokenlists-02
Open

fix: support tokenlists v0.2#72
fubuloubu wants to merge 5 commits into
mainfrom
fix/support-tokenlists-02

Conversation

@fubuloubu

@fubuloubu fubuloubu commented May 30, 2026

Copy link
Copy Markdown
Member

What I did

fixes: #

How I did it

How to verify it

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)

@fubuloubu
fubuloubu force-pushed the fix/support-tokenlists-02 branch from 3cb3325 to 748f158 Compare June 10, 2026 22:00
@fubuloubu
fubuloubu requested a review from antazoey June 10, 2026 22:02
Comment thread ape_tokens/managers.py
return manager

def __repr__(self) -> str:
return f"<ape_tokens.TokenManager default='{self._manager.default_tokenlist}'>"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you may want to check that this still works, I think v2 removed default_tokenlist

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good find! I missed this one

Comment thread ape_tokens/managers.py
def __len__(self) -> int:
tokenlist = self._manager.get_tokenlist()
return len(tokenlist.tokens)
return sum(len(tl.tokens) for tl in self._manager.installed_tokenlists.values())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this means that __len__ and __iter__ differ, meaning if you counted all the items from iterating, it would be different.. kinda a weird design. Iter filters on chain and len doesn't, it seems. I could be wrong

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was thinking about just removing it, it sort of works like a mapping (symbol => token) instead of a list anyways

Comment thread ape_tokens/config.py
default: str | None = None
default: str | None = None # TODO: Remove in v1.0
required: list[ListInfo] = []
order: list[str] | None = None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should add validation here to make the names are actually installed

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I should probably add that upstream

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