This is how current API of System.Data is designed. It uses boxing in many places. I think that this methods should be removed from the base class and be implemented in DataFrameColumn<T>.

They are overridden in DataFrameColumn at the moment?. Do you mean they should occur only in DataFrameColumn and not in the base class? I'm not completely convinced about that yet. At the moment, the DataFrame class has a DataFrameTable which has an IList called columns i.e. the DataFrameTable does not know the real type of the columns it holds => therefore, the approach at the moment is to add APIs on BaseDataFrameColumn and override them in the derived columns. It is possible that this will change down the line as I add more features, but for the moment I think this looks reasonable. Thoughts?

Okay, let's change it a bit later.

Invert condition and use continue.

I'm not opposed to the style but I do have some questions here for the larger crowd:

If I had a DataFrameColumn<int> and called DataFrameColumn<int>.Add(5.0),
The lines
DataFrameColumn<T> column = _table.Column(ii) as DataFrameColumn<T>; if (column != null)

mean that nothing will happen since the dynamic cast will fail. I'm leaning towards checking the type of the resulting operation and creating a new column with that type?
For ex: DataFrameColumn.Add(5.0) will result in a new DataFrameColumn, similar to what 1+5.0 does in C#.

For cases such as DataFrameColumn<int> - string, we just don't perform the operation instead of throwing?
Thoughts?

I would expect it to throw if I try an operation that isn't supported, but why wouldn't that one follow the same rule as number + string with scalars? i.e. convert to string

Ugh, my bad. I typed in a hurry. I meant to write "DataFrameColumn - string"(Basically whenever we have incompatible operands). Edited it now.

Bad indentation here and above. It should be multiple of 4.

Right, I don't think T4TT engine does a great job with the indentations. I did try that before deciding to stay with this scheme

Why is T limited to value types only?

Because we want this to be a DataFrame of primitive types. Since there's no way at the moment to specify primitive types alone, I put in value types. A DataFrame is used for data wrangling with primitive types, similar to Pandas. Is there a way to add reference types to Pandas/DataFrame that makes sense? For ex, what does it mean to say DataFrameColumn.Max() or DataFrameColumn.Add(5)?

you should be able to store strings in a dataframe column.

I agree. At the moment, my plan is to store strings as bytes so I can still keep the struct constraint

I filed an issue to supports string columns in the DataFrame. Working on a patch for it at the moment

We should come up with a better package/assembly name than Microsoft.Data. That namespace is going to have many things in it, and this work shouldn't try taking the whole thing.

Maybe Microsoft.DataFrames?

is there going to be more than one DataFrame? What about System.Data,Frame?

I don't think there will be more than one DataFrame (at least not owned/maintained/worked on by the corefx team at Microsoft).

We could consider putting this in System, but I don't think the type would ever be in the .NET Core shared framework or in the netstandard specification. I imagine this type to always be available through a NuGet package where we can add more APIs without having to rely on a new .NET Core version/TFM. With that in mind, it is unclear to me what should go into a System namespace and what shouldn't.

DataFrame is yet another abstraction over data in general and I think putting it in System is better than putting it in Microsoft. namespace, when other data-related abstractions are in System.Data, etc. It is not windows-specific, and should not be seen as such to encourage wider adoption.

Maybe this should be PrimitiveDataFrameColumn<T> if T must be a struct. With the current name, it feels like DataFrameColumn<string> should be possible, but isn't.

Unless, of course, we remove the T : struct restriction, then I think the current name would make sense.

In reply to: 273526844 [](ancestors = 273526844)

No, I think the T : struct restriction is required. If there was a restriction that truly only allowed primitive types, I would've used that. The DataFrame can only hold primitive types. No ref types and ideally no user defined structs/value types

What is a reason of that restriction? string isn't a primitive type.

This has been renamed to PrimitiveColumn, so the T : struct remains. Strings will be supported with a new column type

Why isn't the backing store for this a Memory<T>?

Because I was going for similarity with Arrow. This way, the Memory can just be wrapped in an Arrow Buffer with no conversion overhead

Is this going to be compatible with Arrow? I've seen the dependency commented out, but I'm not sure whether Arrow is going to be supported as the engine behind the interface.

Yes, this is going to be compatible with Arrow but, if I understand you right, not exactly in the way you mention. The C# Arrow implementation as it exists now requires that the buffer storing the data is immutable. No APIs exist to mutate the contents of an ArrowBuffer. I had a discussion in the Arrow repo a month or so back about removing this restriction, but a lot of stuff has already been built with immutability in mind, so I think it's safe to say that that is a no-go.

By compatible here I mean that a DataFrameBuffer can be wrapped in an ArrowBuffer easily with no overhead. That way the DataFrameBuffer can be mutated by the DataFrame, and anyone who consumes an ArrowBuffer can also get to the data.

The issue is below:

public Span<T> Span
        {
            [MethodImpl(MethodImplOptions.AggressiveInlining)]
            get => MemoryMarshal.Cast<byte, T>(Memory.Span);
        }

This isn't a free operation. If someone calls this many times in a hot path, this Cast is going to take cycles, when if it was just in terms of T to begin with, there is no need for a cast.

Similarly, you wouldn't need to hold onto _size and do division for things like Capacity and MaxCapacity.

I created an issue for this

PrimitiveDataFrameColumn feels very verbose for such a common thing. It would be good to have some shortcuts to create them, like some static functions that could at least infer the generic type and maybe just be called Column()

nit: remove empty lines

This code-path could be optimized. Similar to what was done in List.Add. Or is it too early for such kind of things?

Note: As part of perf work

Use the (ugly) ThrowHelper-pattern, to move the throwing code out of the hot-path, thus allowing the indexer to be inlineable.

Other places as well.

Note you only do this when it is definitely hot path .. we are all assuming the JIT will do this one day

Note: As part of perf work

We may need a little more tests in this PR. This doesn't seem like enough given the functionality in the library with this PR.

I sort of agree. With this patch, the only ops allowed are things like floatColumn + float, so the scope is pretty narrow. Would you consider waiting till the next patch(will be up in an hour) where I've added a couple more tests for binary ops? That patch tests different functions and more combinations such as int + bool/double/decimal

This is OK for now. Let's be sure to add adequate tests in future PRs.

You should initialize this list to the length capacity. That way it doesn't have to grow while you are adding elements to it.

Even though this is internal, this API usage/pattern is a little unusual:

[(int)nextRowIndex, length - ret.Count, ret];

Specifically, passing in an object ret to get filled up. It feels like it should just be a standard method call.

Right. I don't like that I have to do this either. Basically, this is a slice of length "length" that can begin in the middle of some buffer and spill over into the next buffer. I was trying to get by with passing just 1 return list "ret" to both calls to avoid creating more objects. That's why the return type of the indexer is a bool. Ideally, if there was API that made a List from a Span efficiently, or that allowed me to append to a List from a Span, I would've used that. Unless there's a less ugly way to do this? Definitely open to suggestions

I think we can just make this a method and not use an indexer. Honestly, I don't think I've ever seen someone use a C# indexer this way.

Is casting nextRowIndex to int, the right thing here? Same for above where we are casting startIndex to an int. Why take in a long if you are casting it to int?

nextRowIndex should've been 0. That's a bug. Fixing it.

I'm taking a long because the df can hold long number of elements. Each buffer can hold only intMax number of elements though, so I'm casting once I get to the right buffer

        public IList<T> this[long startIndex, int length]
        {
            get
            {
                var ret = new List<T>(length);
                long endIndex = startIndex + length;
                int arrayIndex = GetArrayContainingRowIndex(ref startIndex);
                bool temp = Buffers[arrayIndex][(int)startIndex, length, ret];

Here we take a long startIndex and then are casting it (int)startIndex.

I see that GetArrayContainingRowIndex is decreasing the number, which honestly is not intuitive. Maybe make GetArrayContainingRowIndex return an out int newStartIndex (or a better name). Have GetArrayContainingRowIndex ensure that the number is less than int.MaxValue, then the 3 calling places don't need to cast here. If it ever fails, I'm not sure what would even happen with the current code.

What's the difference between a DataFrame and a DataFrameTable?

Maybe it would be good to have high-level class summary comments describing each public class. You can write them like this:

    /// <summary>
    /// High level summary comment
    /// </summary>
   public class Foo

A DataFrameTable is just a convenient store to hold the columns. The DataFrame class uses it to manipulate the columns and implement its algorithms. I changed the access modifier to internal now so users can only create a DataFrame and don't know anything about the Table

Are we actually using CsvHelper? If not, can you delete this line?

Good catch. Deleted it. I put this in a long time ago to write unit tests

From a public (or a user's) perspective, what is the difference between PrimitiveDataFrameColumn and PrimitiveDataFrameColumnContainer?