Closed Prudhvivuda closed 1 week ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:02Z ----------------------------------------------------------------
This is all based off of just pull request contributors, either change to use all contribution types or change the name
Prudhvivuda commented on 2024-08-26T21:49:20Z ----------------------------------------------------------------
Changed the name
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:03Z ----------------------------------------------------------------
Add to table of contents direct to the networkx graph and plotly for each repository set, Also add the acronym next to it as well if you are going to use it later in the notebook
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:03Z ----------------------------------------------------------------
Please add a more in depth introduction. What are the categories? How were they determined? What Graphs are being plotted? Explain the work that was done in the first notebook and how it will be applied to each group
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:04Z ----------------------------------------------------------------
For each section following as well: Put the name of the category in the graph title
Prudhvivuda commented on 2024-08-27T16:16:51Z ----------------------------------------------------------------
okay
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:05Z ----------------------------------------------------------------
For each section following as well: Put the name of the category in the graph title
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:06Z ----------------------------------------------------------------
Remove the warning
Prudhvivuda commented on 2024-08-26T22:13:32Z ----------------------------------------------------------------
removed.
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:06Z ----------------------------------------------------------------
take out the print of this output
Prudhvivuda commented on 2024-08-26T21:52:31Z ----------------------------------------------------------------
done
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:07Z ----------------------------------------------------------------
take out the print of this output
Prudhvivuda commented on 2024-08-26T21:51:48Z ----------------------------------------------------------------
done
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-26T21:29:08Z ----------------------------------------------------------------
Math - this doesnt match the graph above
NLP- First bullet point just states what the projects are not anything from the graphs in this notebook.
Personal Assistants- "represented in the top contributors" what is this referring to? The point that I think you are trying to say could be supported by the graph itself
Gen AI- I dont see PyTorch or Transformers on the graph
Add takeaways below each graph in their relevant sections
Also, there is more to these graphs than just the relationship between two repositories on them. Take some time to interpret the graphs in their entirety and look at sets of projects not just a single node and edge pair. Most of the take aways you list are just summaries of two projects on the graph and a vague statement about the nature of their connection
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:50Z ----------------------------------------------------------------
Reformat to the following style:
For all of the sections
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:50Z ----------------------------------------------------------------
Reformat for this one and all following "Math Repositories: Networkx Graph"
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:51Z ----------------------------------------------------------------
Reformat for this one and all following "Math Repositories: Plotly Graph"
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:52Z ----------------------------------------------------------------
Much better
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:52Z ----------------------------------------------------------------
much better
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:53Z ----------------------------------------------------------------
++ great change
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:54Z ----------------------------------------------------------------
In this section the addition context provides useful information to help someone understand the graphs better. Since there is really only one influential pair this makes sense for this graph
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-08-29T20:02:55Z ----------------------------------------------------------------
Put in the write ups you have under each graph into this section
Prudhvivuda commented on 2024-08-31T00:32:26Z ----------------------------------------------------------------
Should I just copy the takeaways from each section and paste it here?
cdolfi commented on 2024-09-03T19:51:32Z ----------------------------------------------------------------
yes
Prudhvivuda commented on 2024-09-03T20:29:08Z ----------------------------------------------------------------
done
@Prudhvivuda Great job, lot of improvement this review cycle
Should I just copy the takeaways from each section and paste it here?
View entire conversation on ReviewNB
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:12Z ----------------------------------------------------------------
Does this include both open and closed PRs?
Prudhvivuda commented on 2024-09-17T21:43:32Z ----------------------------------------------------------------
Yes, it includes both open and closed PRs.
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:12Z ----------------------------------------------------------------
can you print a few items of the contributorGraph
dictionary just to see what it looks like?
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:13Z ----------------------------------------------------------------
same as above comment, can you print this?
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:14Z ----------------------------------------------------------------
same as above comment, could you print this out?
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:15Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:15Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:16Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:17Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:18Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:18Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:19Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:20Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:21Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:21Z ----------------------------------------------------------------
similar to above comments
cdolfi commented on 2024-09-17T10:14:38Z ----------------------------------------------------------------
Does every key value pair only have 1 repo? If not show an output of examples with variable length
Prudhvivuda commented on 2024-09-17T20:56:53Z ----------------------------------------------------------------
No. There are key value pairs with more than 1 repo. Changed the code to print first 15 items. Now it contains at least 1 item with more than 1 repo as a value for a key.
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:22Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:23Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:23Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:24Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:25Z ----------------------------------------------------------------
similar to above comments
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:25Z ----------------------------------------------------------------
I don't see a graph being rendered here in the notebook, is that expected?
Prudhvivuda commented on 2024-09-17T21:45:52Z ----------------------------------------------------------------
This was expected with the old code. I rewrote small code to write the image to a file and display. Now, it should render in the notebook.
View / edit / reply to this conversation on ReviewNB
hemajv commented on 2024-09-09T23:40:26Z ----------------------------------------------------------------
similarly here, I don't see a graph being rendered
Does every key value pair only have 1 repo? If not show an output of examples with variable length
View entire conversation on ReviewNB
View / edit / reply to this conversation on ReviewNB
cdolfi commented on 2024-09-17T10:17:06Z ----------------------------------------------------------------
Does every key value pair only have 1 repo? If not show an output of examples with variable length
Prudhvivuda commented on 2024-09-17T20:56:44Z ----------------------------------------------------------------
No. There are key value pairs with more than 1 repo. Changed the code to print first 15 items. Now it contains at least 1 item with more than 1 repo as a value for a key.
In this notebook, we will try to plot the graphs for each category(math, nlp, llm, gen_ai, personal_assistants) in the AI/ML we defined earlier.