Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added price prediction model #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StunningShield4504
Copy link

This model is made by collecting official data from government websites and based on rainfall and temperature it will predict the future price of commodities.Just we have to add it on the main page that ill do it in one or two days.

This model is made by collecting official data from government websites and based on rainfall and temperature it will predict the future price of commodities.
@StunningShield4504
Copy link
Author

@yashasvini121 Can you please review this and suggest me some improvements if required?

@yashasvini121 yashasvini121 self-requested a review October 4, 2024 18:05
Copy link
Owner

@yashasvini121 yashasvini121 left a comment

Choose a reason for hiding this comment

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

First of all, nice idea!! Your code seems great, but I believe, a few changes would make it better.

To implement the suggested changes, please follow these steps:

  1. Sync your fork and pull the branch accordingly.
  2. To update your code, you need to make the necessary modifications directly in your current working branch.
  3. Apply the changes as discussed in the reviews, following all the 🔴 Steps.
  4. Once you've made the changes and committed them, the pull request will automatically reflect the updates you've made.

According to me, these reviews should work and not give you any bugs. Let me know if you still get any errors while correcting the project structure.

st.sidebar.markdown("<h2 style='color: #0056b3;'>User Inputs</h2>", unsafe_allow_html=True)

with st.sidebar.expander("Select Parameters", expanded=True):
state = st.selectbox("Select State", ["Mumbai"],
Copy link
Owner

Choose a reason for hiding this comment

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

🔴 Step 6:

Only a single state is available, see If you have any unused files, or options. Or maybe you plan on working on them later on.

Make the changes accordingly.

Copy link
Owner

Choose a reason for hiding this comment

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

🔴 Step 2:

This file is not being utilized anywhere, according to me. Please verify whether it should be removed.
Similarly check for the Mumbai_moong.py and Mumbai_urad.py files.

Copy link
Owner

Choose a reason for hiding this comment

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

This file is not being utilized anywhere. Please verify whether it should be removed.

Copy link
Owner

Choose a reason for hiding this comment

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

This file is not being utilized anywhere. Please verify whether it should be removed.

Copy link
Owner

Choose a reason for hiding this comment

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

🔴 Step 1:

  1. Please check if this file is being utilized anywhere in the project.
  2. Convert this file to CSV format because it is generally more efficient for data storage and compatibility with various data processing tools, and can be viewed by the developer easily.
  3. Update the relevant code in the project, where you are using pd.read_excel() function.

This review is for all the files in data/

Copy link
Owner

Choose a reason for hiding this comment

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

🔴 Step 5:

To align the application with the project structure, please follow these steps:

  1. Change the file name from app.py to Mumbai_Agriculture_Price_Prediction.py.
  2. Transfer the renamed file to the pages/ folder.
  3. Adjust the file paths for data files to reflect their new locations.
  4. Modify any module import paths as necessary to ensure they align with the new file structure.


# Load data
@st.cache_data
def load_data(file_path):
Copy link
Owner

@yashasvini121 yashasvini121 Oct 4, 2024

Choose a reason for hiding this comment

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

🔴 Step 3:

  • Please move the specified code to a separate file within the existing folder to enhance modularity and maintainability in the codebase.
  • Then update the module paths in App.py accordingly to reflect the changes.



# Prediction logic
def predict_prices(df, commodity, days_to_predict, rainfall, temperature):
Copy link
Owner

@yashasvini121 yashasvini121 Oct 4, 2024

Choose a reason for hiding this comment

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

🔴 Step 4:

  • Please move this code to a separate file within the existing folder to enhance modularity and maintainability in the codebase.
  • Then update the module paths in App.py accordingly to reflect the changes.

@import url('https://fonts.googleapis.com/css2?family=Roboto:wght@400;500;700&display=swap');

.main {
background-image: url("https://wallpapercave.com/wp/wp9212753.jpg");
Copy link
Owner

Choose a reason for hiding this comment

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

🔴 Step 7:

See if the image is working out. If not then remove it.

@yashasvini121
Copy link
Owner

@StunningShield4504, updates?

@StunningShield4504
Copy link
Author

@yashasvini121 Its done ill push it by tomorrow
Sorry for delay due to my exams,they are going on!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Changes Requested
Development

Successfully merging this pull request may close these issues.

Adding agricultural commodities price prediction model
2 participants