StephanRhode / py-algorithms-4-automotive-engineering

This repository contains jupyter notebooks and python code for KIT course: Python Algorithms for Automotive Engineering
http://www.fast.kit.edu/lff/1017_13056.php
MIT License
63 stars 95 forks source link

DeepL notebook #12

Closed mauricio-fernandez-l closed 4 years ago

mauricio-fernandez-l commented 4 years ago

Deep learning lecture is ready. A folder 09_files contains all the supporting files (some figures, scripts and training data for students).

StephanRhode commented 4 years ago

Thanks Mauricio, I will review it the next days. At first look, there are two small merge conflicts which prevents to run the tests on travis CI.

StephanRhode commented 4 years ago

The merge conflict is resolved and there is now a file not found issue on Travis CI. I had the same problem with some other notebook where I read a csv file. In Jupyter, relative file paths work. Pytest is running on Travis, the path is different because pytest starts from root folder. I used a workaround like this to fix this issue in other notebooks:

# This if else is a fix to make the file available for Jupyter and Travis CI
import os

if os.path.isfile('my_file.csv'):
    file = 'my_file.csv'
else:
    file = '02_tools-and-packages/my_file.csv'

You can test pytest locally if you just type pytest in terminal from root folder.

I will try to fix your notebook so that it runs locally and on Travis. If you know a better solution than my workaround from above, please let me know.

StephanRhode commented 4 years ago

The regression file runs well on my computer but I get a value error from the classification file in line 16 of get_data function. The error is:

labels = np.array(labels,dtype=np.int)[:,np.newaxis] ValueError: invalid literal for int() with base 10: 'data/16'

Also, do you think it makes sense to write two tests for the solution scripts, or actually call them in the notebook (which would test them automatically)?

mauricio-fernandez-l commented 4 years ago

Hmmm... the classification.py script works fine on my end. I think I know what the problem is at your end, if you are working in a Linux system. I am working in Windows where glob imports file paths as strings with \\ in them. I split those in the classification.py script in the line

labels = [file.split('\\')[-1].split('_')[0] for file in files]

which does NOT work if you run the code in Linux, since Linux data/16_3 is not split into 16_3 and then into 16 for the np.array.

Does the test script checks for further Linux and Windows differences? Or do we want to do everything Linux-style for consistency? Do you want me to change it?

With respect to the test: I can incorporate running the scripts in the notebook. It is easier.

StephanRhode commented 4 years ago

Hmmm... the classification.py script works fine on my end. I think I know what the problem is at your end, if you are working in a Linux system. I am working in Windows where glob imports file paths as strings with \\ in them. I split those in the classification.py script in the line

labels = [file.split('\\')[-1].split('_')[0] for file in files]

which does NOT work if you run the code in Linux, since Linux data/16_3 is not split into 16_3 and then into 16 for the np.array.

Does the test script checks for further Linux and Windows differences? Or do we want to do everything Linux-style for consistency? Do you want me to change it?

With respect to the test: I can incorporate running the scripts in the notebook. It is easier.

Good point, this must be the reason. My laptop as well as Travis are Linux. Nope, the tests do not account for Win Linux version. I guess this path split line of code is actually the only place where such an error shows up. Could you please fix it either with some operating system independent code or with a if else which checks first on which system the code is running?

You can double check on Travis if you either write a test for the scripts or call them in your notebook.

awiawi commented 4 years ago

Maybe you can replace file.split('\\')[-1] with os.path.basename(file)? I guess this should work on all systems.

https://docs.python.org/3.7/library/os.path.html#os.path.basename

StephanRhode commented 4 years ago

Maybe you can replace file.split('\\')[-1] with os.path.basename(file)? I guess this should work on all systems.

https://docs.python.org/3.7/library/os.path.html#os.path.basename

Great, this resolves the bug. Thanks!