Ported token mapping function #22
Conversation
scripts/map_tokens.py
Outdated
|
|
||
|
|
||
| def token_map( | ||
| tokenized_dataset: DatasetDict | Dataset | IterableDatasetDict | IterableDataset, |
There was a problem hiding this comment.
The load_dataset function returns one of those 4 types. Pylance got mad at me if I just use one of the four, so I included all 4 possible return types here. I think our tokenized dataset only has the Dataset class.
There was a problem hiding this comment.
You can try using https://docs.python.org/3/library/typing.html#typing.cast let pyright / pylance know we will be only using Dataset only before calling this function
src/delphi/dataset/token_map.py
Outdated
| mapping = {} | ||
| tokenized_dataset = cast(Dataset, tokenized_dataset) | ||
| for prompt_idx, prompt in enumerate(tokenized_dataset): | ||
| prompt = cast(dict, prompt) |
There was a problem hiding this comment.
what's the error here, pyright doesn't realize prompt has a get_item?
There was a problem hiding this comment.
Yes. this is the full message:
Argument of type "Literal['tokens']" cannot be assigned to parameter "__s" of type "slice" in function "__getitem__"
"Literal['tokens']" is incompatible with "slice"
I tested the dataset live, and the prompt seems to be of type dict when I access it, so I casted it to dict. Should I get a typeignore comment in or do something else?
tests/test_dataset.py
Outdated
| tokenized_dataset = Dataset.from_dict( | ||
| { | ||
| "tokens": [ | ||
| [ | ||
| 0, | ||
| 1, | ||
| 2, | ||
| 3, | ||
| 4, | ||
| 5, | ||
| 0, | ||
| 6, | ||
| 7, | ||
| 0, | ||
| 1, | ||
| 2, | ||
| 3, | ||
| 4, | ||
| 5, | ||
| 0, | ||
| 6, | ||
| 7, | ||
| 0, | ||
| 1, | ||
| 2, | ||
| 3, | ||
| 4, | ||
| 5, | ||
| 0, | ||
| 6, | ||
| 7, | ||
| ], | ||
| ] | ||
| } | ||
| ) |
There was a problem hiding this comment.
this is bad xD
you can add a noqa comment for black and keep it at reasonable number of lines
There was a problem hiding this comment.
Hahaha yeah thank you I wasn't sure how to deal with that
|
test looks good! |
|
@jettjaniak I've updated most things, but still a little confused about some things:
|
|
let's discuss at check-in |
menamerai
force-pushed
the
8-map-tokens-to-prompts-and-positions
branch
from
94fa5f1 to
3dedb4c
Compare
Ported mapping function over from previous repo. Made modifications so it works with the HF Dataset classes.
For issue #8.