Skip to content

Conversation

Amit366
Copy link
Contributor

@Amit366 Amit366 commented Mar 11, 2021

Description

The script will compare two spreadsheets and then show a pie chart

Fixes #469

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congratulations!! for making your first PR at Amazing-Python-Scripts, our mentors will review it soon.

@Amit366
Copy link
Contributor Author

Amit366 commented Mar 11, 2021

@avinashkranjan please add the level tag

Copy link
Contributor

@antrikshmisri antrikshmisri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amit366 Please add a README.md with instruction on how to run the script

Copy link
Contributor

@antrikshmisri antrikshmisri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved from my side. @kaustubhgupta please add the tags

Copy link
Contributor

@kaustubhgupta kaustubhgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a generalized script. What happens if I select totally different datasets that do not have "Total Price" and "Purchased Amount" columns? Here is the solution to this:

  • Make it a CLI version where the user mentions the dataset 1 column to be compared and the same for dataset 2.
  • Take the input for the common column where the merger will happen.
  • Now display the plots

@kaustubhgupta kaustubhgupta added the bug Something isn't working label Mar 12, 2021
@Amit366
Copy link
Contributor Author

Amit366 commented Mar 12, 2021

@kaustubhgupta sir I have updated it

item = input("What is the basis of merging? ")
data_total = data_home_1.merge(data_prices, on=item)

data_total['Total Price'] = data_total['PURCHASED AMOUNT'] * data_total['Price']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not required I think?

data_total['Total Price'] = data_total['PURCHASED AMOUNT'] * data_total['Price']

#print​(df_total)
material=input("Enter criteria 1")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name the variables with descriptive meaning. Suggestion:

Suggested change
material=input("Enter criteria 1")
criteria_1=input("Enter criteria 1")

#print​(df_total)
material=input("Enter criteria 1")
price=input("Enter criteria 2")
fig = px.pie(data_total[[material, price]], values=price, names=material)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the changes here also

@Amit366
Copy link
Contributor Author

Amit366 commented Mar 12, 2021

@kaustubhgupta now check

Copy link
Contributor

@kaustubhgupta kaustubhgupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't notice these variables earlier:

import plotly.express as px

# storing the dataset
book_relative_path = input("Enter first dataset")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two datasets names also need to be changed to something good.

book_prices = input("Enter second dataset")

# reading the data
data_prices = pd.read_excel(book_prices)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also


#print​(df_prices, df_home_1)

item = input("What is the basis of merging? ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this item too.

@Amit366
Copy link
Contributor Author

Amit366 commented Mar 13, 2021

@avinashkranjan updated

@kaustubhgupta kaustubhgupta removed the bug Something isn't working label Mar 13, 2021
@kaustubhgupta
Copy link
Contributor

@santushtisharma10 your approval awaited

@kaustubhgupta kaustubhgupta added the next review needed Approved by some mentors, more approvals needed label Mar 13, 2021
@kaustubhgupta kaustubhgupta added Approved PR Approved and Ready to Merge gssoc23 Issues created for/by the GirlScript Summer of Code'23 Participants level2 Bug fixing, Adding small features and removed next review needed Approved by some mentors, more approvals needed labels Mar 14, 2021
@avinashkranjan avinashkranjan merged commit 573b440 into avinashkranjan:master Mar 14, 2021
@avinashkranjan
Copy link
Owner

@all-contributors please add @Amit366 for code and documentation

@allcontributors
Copy link
Contributor

@avinashkranjan

I've put up a pull request to add @Amit366! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved PR Approved and Ready to Merge gssoc23 Issues created for/by the GirlScript Summer of Code'23 Participants level2 Bug fixing, Adding small features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spreadsheet Automation
5 participants