Skip to content

/ Python

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 Lstm example for stock predection #1908

Merged
merged 6 commits into from May 7, 2020
Merged

Added Lstm example for stock predection #1908

merged 6 commits into from May 7, 2020

Conversation

@jeffin07
Copy link
Contributor

jeffin07 commented Apr 26, 2020

Describe your change:

PR for the issue #1770
I have created a small LSTM network for stock prediction.The data used are also included

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.
@jeffin07
Added Lstm example for stock predection
Loading status checks…
b49c1f9
@cclauss
cclauss requested changes Apr 26, 2020
View changes
machine_learning/lstm/lstm_prediction.py Outdated
from keras.models import Sequential
from keras.layers import LSTM,Dense

def clean_data(data_file):

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020 •
edited

Member

No type hints, no doctests. See CONTRIBUTING.md.

Is this flexible to clean multiple datasets or is it hardwared to this particular dataset? If flexible, what other dataset could the reader consider using?
machine_learning/lstm/lstm_prediction.py Outdated

def clean_data(data_file):
'''
This function will read and clean data

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

We can lose This function will.
machine_learning/lstm/lstm_prediction.py Outdated
output: Data which will be useful for training
'''
df = pd.read_csv(data_file, header=None)
print(df.head())

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

Algorithm functions should not print(). See CONTRIBUTING.md.
machine_learning/lstm/lstm_prediction.py Outdated
len_data = df.shape[:1][0]
actual_data = df.iloc[:,1:2]
actual_data = actual_data.values.reshape(len_data,1)
scl = MinMaxScaler()

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

scl --> min_max_scaler (don't make your reader guess) or just do MinMaxScaler().fit_transform(actual_data) if it is a one-time use.
machine_learning/lstm/lstm_prediction.py Outdated
print(train_data,test_data)
train_x,train_y=[],[]
test_x,test_y=[],[]
for i in range(0,len(train_data) - forward_days - look_back + 1,1):

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

Why start with zero and end with one. These are not needed and only distract your reader. Same a few lines below.
machine_learning/lstm/lstm_prediction.py Outdated
print(x_train.shape)
print(x_test.shape)
print(y_train.shape)
print(y_test.shape)
Comment on lines 40 to 43

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

Algorithm functions should not print(). See CONTRIBUTING.md.
machine_learning/lstm/lstm_prediction.py Outdated


if __name__ == '__main__':
x_train,y_train,x_test,y_test = clean_data('google_data.csv')

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020 •
edited

Member

Where does google_data.csv come from and under what license is it published?

Strong preference would be that this data is downloaded in the __main__ code block rather than being stored in this repo.

This comment has been minimized.

Sign in to view
Copy link
@jeffin07

jeffin07 Apr 27, 2020

Author Contributor

I have used this Link to get the data
machine_learning/lstm/lstm_prediction.py Outdated
@@ -0,0 +1,66 @@
import pandas as pd

This comment has been minimized.

Sign in to view
Copy link
@cclauss

cclauss Apr 26, 2020

Member

It would be helpful if this file defined what LSTM is. Or at least pointed to an article like https://en.wikipedia.org/wiki/Long_short-term_memory to so than readers do not need to guess.

The only function in this submission is clean_data() so is this the submission of an example (as the PR title says) of how to use Python packages or is there an algorithm here? See CONTRIBUTING.md's What is an algorithm?

This comment has been minimized.

Sign in to view
Copy link
@jeffin07

jeffin07 Apr 27, 2020

Author Contributor

This is an example of LSTM stock prediction using Keras,Which is a deep-learning library
@cclauss
Copy link
Member

cclauss commented Apr 26, 2020

https://github.com/TheAlgorithms/Python/blob/master/Travis_CI_tests_are_failing.md
@jeffin07
Changes after review
Loading status checks…
ad96d20
@jeffin07
Copy link
Contributor Author

jeffin07 commented Apr 27, 2020

Done Changes after the review
@jeffin07
changes after build failed
Loading status checks…
71c045a
@mrmaxguns
Copy link
Contributor

mrmaxguns commented May 6, 2020

Hi there, I just wanted to let you know why the build is failing.

The build is failing due to an ModuleNotFoundError. When the travis ci's virtual machine boots, it first installs all the needed dependencies specified in requirements.txt. keras is a third party package and not installed in the python standard library, nor was it specified in requirements.txt. All third party packages must be declared in requirements.txt so that developers and automated testing know what to install.

This is what CONTRIBUTING.md says:

If you need a third party module that is not in the file requirements.txt, please add it to that file as part of your submission.

Assuming your module requires the latest version of keras you will need to add keras to requirements.txt. I hope that clears some things up.
cclauss added 2 commits May 7, 2020
@cclauss
Add Kiera’s to requirements.txt
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
06a2423
@cclauss
requirements.txt: Add keras and tensorflow
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
bdddbd0
@cclauss
Copy link
Member

cclauss commented May 7, 2020 •
edited

I think the problem is that keras requires tensorflow
and tensorflow does not yet work with Python 3.8.

Update: this was fixed in TensorFlow 2.2.0 which was released 11 hours ago.
@cclauss
psf/black
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c86f555
@cclauss
cclauss approved these changes May 7, 2020
View changes
@cclauss cclauss merged commit 8a8527f into TheAlgorithms:master May 7, 2020
1 of 2 checks passed
1 of 2 checks passed
codespell
Details
Travis CI - Pull Request Build Errored
Details
@cclauss
Copy link
Member

cclauss commented May 7, 2020

Thanks for your contribution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cclauss cclauss

Assignees
No one assigned
Labels
automated tests are failing
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@jeffin07 @cclauss @mrmaxguns
You can’t perform that action at this time.