Bug Report: timestamp[ns] not recognized
State: closed | Author: justin-yan (CONTRIBUTOR)

Repro:

```
from datasets import Dataset
import pandas as pd
import pyarrow

df = pd.DataFrame(pd.date_range("2018-01-01", periods=3, freq="H"))
pyarrow.Table.from_pandas(df)
Dataset.from_pandas(df)
# Throws ValueError: Neither timestamp[ns] nor timestamp[ns]_ seems to be a pyarrow data type.
```

The factory function seems to be just "timestamp": https://arrow.apache.org/docs/python/generated/pyarrow.timestamp.html#pyarrow.timestamp

It seems like https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L36-L43 could have a little bit of additional structure for handling these cases?  I'd be happy to take a shot at opening a PR if I could receive some guidance on whether parsing something like `timestamp[ns]` and resolving it to timestamp('ns') is the goal of this method.

Alternatively, if I'm using this incorrectly (e.g. is the expectation that we always provide a schema when timestamps are involved?), that would be very helpful to know as well!

```
$ pip list  # only the relevant libraries/versions
datasets                      1.2.1
pandas                        1.0.3
pyarrow                       3.0.0
```

Comments: Thanks for reporting !

You're right, `string_to_arrow` should be able to take `"timestamp[ns]"` as input and return the right pyarrow timestamp type.
Feel free to suggest a fix for `string_to_arrow` and open a PR if you want to contribute ! This would be very appreciated :)

To give you more context:

As you may know we define the features types of a dataset using the `Features` object in combination with feature types like `Value`. For example
```python
features = Features({
    "age": Value("int32")
})
```
However under the hood we are actually using pyarrow to store the data, and so we have a mapping between the feature types of `datasets` and the types of pyarrow.

For example, the `Value` feature types are created from a pyarrow type with `Value(str(pa_type))`.
However it looks like the conversion back to a pyarrow type doesn't work with `"timestamp[ns]"`.
This is the `string_to_arrow` function you highlighted that does this conversion, so we should fix that.



Thanks for the clarification @lhoestq !

This may be a little bit of a stupid question, but I wanted to clarify one more thing before I took a stab at this:

When the features get inferred, I believe they already have a pyarrow schema (https://github.com/huggingface/datasets/blob/master/src/datasets/arrow_dataset.py#L234).

We then convert it to a string (https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L778) only to convert it back into the arrow type (https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L143, and https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L35).  Is there a reason for this round-trip?

I'll open a PR later to add `timestamp` support to `string_to_arrow`, but I'd be curious to understand since it feels like there may be some opportunities to simplify!

The objective in terms of design is to make it easy to create Features in a pythonic way. So for example we use a string to define a Value type.
That's why when inferring the Features from an arrow schema we have to find the right string definitions for Value types. I guess we could also have a constructor `Value.from_arrow_type` to avoid recreating the arrow type, but this could create silent errors if the pyarrow type doesn't have a valid mapping with the string definition. The "round-trip" is used to enforce that the ground truth is the string definition, not the pyarrow type, and also as a sanity check.

Let me know if that makes sense 

OK I think I understand now:

Features are datasets' internal representation of a schema type, distinct from pyarrow's schema.
Value() corresponds to pyarrow's "primitive" types (e.g. `int` or `string`, but not things like `list` or `dict`).
`get_nested_type()` (https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L698) and `generate_from_arrow_type()` (https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L778) *should* be inverses of each other, and similarly, for the primitive values, `string_to_arrow()` and `Value.__call__` (https://github.com/huggingface/datasets/blob/master/src/datasets/features.py#L146) should be inverses of each other?

Thanks for taking the time to answer - I just wanted to make sure I understood before opening a PR so I'm not disrupting anything about how the codebase is expected to work!

Yes you're totally right :)