Extension for DataFrame + Jupyter notebooks that adds properties that return concrete columns #25

pgovind · 2020-02-11T23:25:52Z

Also spits out the API for the generated type. Looks like this at the moment:

The magic command also takes in an optional TypeName OutputType to generate public class OutputType: DataFrame instead of the default InteractiveDataFrame.

cc: @eiriktsarpalis

pgovind · 2020-02-11T23:35:02Z

Just after I put this up, Jon pinged me about how to make the following option work in the magic command:

#!AddStrongTypes DataFrame df

I'll update the PR for it soon, but that should be a minor update

eiriktsarpalis · 2020-02-12T11:46:11Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

            {
-                Handler = CommandHandler.Create(async (FileInfo csv, string typeName, KernelInvocationContext context) =>
+                Handler = CommandHandler.Create(async (string dataFrameName, string typeName, KernelInvocationContext context) =>


I'm curious, why did you remove the csv argument from the command arguments? The current approach requires 3 commands to initialize a frame with strongly typed columns, shouldn't it be possible to do it with just one?

Agreed I think we took a step back in usability here. I think it is a good addition to add the option to take an existing DataFrame as input, as that could be useful. But I would expect a "simple" case to still be possible.

My thinking on the simple case would be:

#!LoadDataFrame csv C:\blah\housing.csv

housing.Description()

Note, we can try to infer both the generated type and the variable name from the file name. In this case, we would generate HousingDataFrame Type and housing variable which is populated with the data from that file.

Hmmm, my initial thought was for the magic command to generate strongly typed columns on any DataFrame. So in my head, I kinda separated out reading a csv file from generating a new type. I'll add the csv option back in.

Alright, so our LoadCsv method can sometimes fail to infer the correct types. In these cases, this extension will generate the wrong column types for it's properties, but this is ok since we can pass in a regular DataFrame and generate strong properties on it.

eiriktsarpalis · 2020-02-12T11:47:57Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

-                "typeName",
-                getDefaultValue:() => "Foo"));
+                "--type-name",
+                getDefaultValue: () => "InteractiveDataFrame"));


Wouldn't using a constant as the default type name result in shadowing of any previous InteractiveDataFrame declarations? The default should probably be qualified by some kind of cell identifier.

It does result in shadowing but I figured that would be the expected behavior, since everything else in a notebook behaves this way. Adding a cell identifier would make that more obvious, you're right. I'll look into what identifier information is available here and update this code.

This is tricky. Cell identifiers aren't something you can consistently get access to across Juptyer frontends, despite various workarounds that exist to accomplish this. Remember that cells are transient. Submissions are more important in the interactive session, and the types will align to the specific submissions. In some languages you won't be able to access shadowed types except via closure, so this can also be hard to reason about.

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

src/Microsoft.Data.Analysis.Interactive/Microsoft.Data.Analysis.Interactive.csproj

eerhardt · 2020-02-12T16:58:50Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

+    {{
+        get
+        {{
+            int columnIndex = Columns.IndexOf(""{columnName}"");


Why isn't this just return Columns[""{columnName}""];?

Because Columns[columnName] doesn't exist on the DataFrame side in 0.2.0.

Then why not return this[""{columnName}""];?

Because return this["columnName"] will go away in the next preview in favor of Column[columnName].

eerhardt · 2020-02-12T17:00:05Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

+                                {
+                                    string typedDataFrame = GetTypedDataFrameWithProperties(df, typeName, out StringBuilder prettyFormatter);
+                                    context.Publish(new DisplayedValueProduced($"Created type {typeName}: ", context.Command));
+                                    await context.DisplayAsync(prettyFormatter.ToString());


Why are we displaying different code than what we are generating/compiling?

I thought this looked better since we're only seeing the API rather than spitting out the code being compiled. Would you rather see the implementation too? I tried that first and it looked rather noisy. I would think most folks aren't interested in looking at the implementation?

My thinking is that someone could use this tool to generate code that they can use in their project and use it in the library/application. Not just in a notebook.

Outputting the generated code could be an option that is off by default.

eerhardt · 2020-02-24T20:23:28Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs


 namespace Microsoft.Data.Analysis.Interactive
 {
    public class DataFrameKernelExtension : IKernelExtension
    {
-        public Task OnLoadAsync(IKernel kernel)
+        bool _generateCsvMethod = false;


Would it be better to flow this bool down through the method calls as a parameter, instead of setting a field property here? What happens if multiple threads are calling this object at the same time?

eerhardt · 2020-02-24T20:27:31Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

+                {
+                    friendlyName = friendlyName.Remove(backTick);
+                }
+                friendlyName += "<";


It feels like there should be an existing utility function somewhere that does this: Given a Type, write out its language specific name.

Note that as written, this will only work for C#.

eerhardt · 2020-02-24T20:30:14Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

+                    {
+                        if (csv != null)
+                        {
+                            HandleCsvAsync(csv, context);


These HandleXXXAsync calls need to be await'd.

eerhardt · 2020-02-24T20:31:10Z

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs

+                    {
+                        if (csv != null)
+                        {
+                            HandleCsvAsync(csv, context);


Why doesn't HandleCsvAsync get the --type-name and --dataframe-name parameters passed to it?

pgovind requested a review from eerhardt February 11, 2020 23:35

eiriktsarpalis reviewed Feb 12, 2020

View reviewed changes

src/Microsoft.Data.Analysis.Interactive/DataFrameKernelExtension.cs Outdated Show resolved Hide resolved

eerhardt reviewed Feb 12, 2020

View reviewed changes

src/Microsoft.Data.Analysis.Interactive/Microsoft.Data.Analysis.Interactive.csproj Outdated Show resolved Hide resolved

eerhardt reviewed Feb 12, 2020

View reviewed changes

Extension to generate properties that return strong columns on DataFrame

1a37a94

pgovind force-pushed the dataframe_interactive branch from fdd83dd to 1a37a94 Compare February 19, 2020 23:08

eerhardt reviewed Feb 24, 2020

View reviewed changes

Base automatically changed from master to main March 5, 2021 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extension for DataFrame + Jupyter notebooks that adds properties that return concrete columns #25

Extension for DataFrame + Jupyter notebooks that adds properties that return concrete columns #25

pgovind commented Feb 11, 2020 •

edited

Loading

pgovind commented Feb 11, 2020

eiriktsarpalis Feb 12, 2020

eerhardt Feb 12, 2020

pgovind Feb 12, 2020

pgovind Feb 19, 2020

eiriktsarpalis Feb 12, 2020 •

edited

Loading

pgovind Feb 12, 2020

jonsequitur Feb 12, 2020

eerhardt Feb 12, 2020

pgovind Feb 12, 2020

eerhardt Feb 12, 2020

pgovind Feb 19, 2020

eerhardt Feb 12, 2020

pgovind Feb 12, 2020

eerhardt Feb 12, 2020 •

edited

Loading

eerhardt Feb 24, 2020

eerhardt Feb 24, 2020

eerhardt Feb 24, 2020

eerhardt Feb 24, 2020

Extension for DataFrame + Jupyter notebooks that adds properties that return concrete columns #25

Are you sure you want to change the base?

Extension for DataFrame + Jupyter notebooks that adds properties that return concrete columns #25

Conversation

pgovind commented Feb 11, 2020 • edited Loading

pgovind commented Feb 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eiriktsarpalis Feb 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eerhardt Feb 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgovind commented Feb 11, 2020 •

edited

Loading

eiriktsarpalis Feb 12, 2020 •

edited

Loading

eerhardt Feb 12, 2020 •

edited

Loading