Teradata / jupyter-demos

16 stars 19 forks source link

Product Engineering Review: Graph Analysis #533

Open ForbiddenDevil opened 7 months ago

ForbiddenDevil commented 7 months ago

Reviewer 1 comments:

Reviewer 1 suggestions:

Image

Reviewer 2 comments:

ForbiddenDevil commented 5 months ago

Incorporated suggested changes. PR: https://github.com/Teradata/jupyter-demos/pull/591

Reviewer 1 comments:

  • Warnings should not be ignored --- This is a standard used in all the notebooks to keep the notebook clean.
  • Section 1.3

    • procedure call is used for loading the data --- Again, this is the standard for all the notebooks.
  • Section 2.1

    • Used sql query for Script and procedure calls for install and remove file --- Fixed this, thank you.
    • Section 2.2, 2.3, 2.4 same comment as 2.1 --- Fixed this, thank you.
  • Section 3.2

    • used pandas merge() --- Fixed this
    • use of matplotlib. --- Graph plotting functionality with versatile features is missing from teradataml plots.

Reviewer 1 suggestions:

  • Warnings should not be ignored --- This is a standard used in all the notebooks to keep the notebook clean.
  • procedure call is used for loading the data, instead, teradataml copy_to_sql can be used. --- Again, this is the standard for all the notebooks.
  • Instead of using execute_sql for installation and removal of file, One can create Script object that allows us to install_file, execute script on Vantage and removefile. --- Fixed this, thank you._

Image

  • pandas merge:

    • Use teradataml’s merge() instead of pandas --- Fixed this, thank you.
  • matplotlib:

    • use teradataml’s plot functionality instead of --- Graph plotting functionality with versatile features is missing from teradataml plots.

Reviewer 2 comments:

  • All the points covered by Reviewer 1.