Closed Will-Shanks closed 4 years ago
The submission script still needs to be tested, but besides that is should be good to go
The submission script still needs to be tested, but besides that is should be good to go
Submission script works.
Hello @Will-Shanks! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
src/dayToDF.py
:Line 66:9: E265 block comment should start with '# ' Line 69:80: E501 line too long (142 > 79 characters) Line 94:80: E501 line too long (100 > 79 characters)
To make my request clear I think every thing looks good I am just not sure about merging the daysToTitles.sh file as I am not sure we need it.
Im running the rest of the src code now to make sure I can get this to work
On second thought I am going to approve the request and let will do the merge after he check over my comments.
Alrght so I thin the daytoTitles.sh may be needed to run the code on summit.
:)
So I think we should be good to just merge the whole thing then :)
Tried to run this on both my local device as well as Summit and got the sma eerror both times:
Sat Apr 11 09:07:56 MDT 2020
DEBUG:root:Creating DayReader for year 1930
DEBUG:root:finding files for year 1930, at brightness 70, with basedir /scratch/summit/diga9728/Moodys/Industrials/
DEBUG:root:Moving to next page, /scratch/summit/diga9728/Moodys/Industrials/OCRrun1930/000/OCRoutputIndustrial19300001-000170.day
Traceback (most recent call last):
File "/home/keea2562/csci4318/company_names/code/getTitles.py", line 65, in
After some more looking it appears that the issues is that in line 269 of the dayToDF file we are using the next comand to iterate over a df.
However if the df is empty it throws the exception seen above.
I am working on handling this.
I think we should add a comment at the top of each file stating what its dependencies are and what it does.
For instance If I just open the daytotitles file I have no clue what it is supposed to be used for.
Also can I get a description on what we mean by title? Is that the company name?
All of the python files already have a comment at the top that is supposed to describe what its code should do, which is followed by the import statements which are all of a files dependencies. I'll add a comment to the daysToTitles.sh sbatch script describing its purpose.
For the conf.py.
I think this just exists to help build some of the documentation.
But I am not sure.
Yes, everything in the docs dir is for sphinx generated documentation files.
Alright,
After looking through this here is my understanding.
We are merging the code in the src folder to the master branch.
This code has 5 files
dayToDF - draw - getTitles - nav - oneCol -
Correct, It updates a lot of the code
Each of these has a corresponding html doc describing what it does and how to use it.
Then there are a few other HTML files we are merging as well. genindex index modules pymodindex search
These all look like they exist to make the structure of the document database WIll made. No issues here everything looks good.
Then there is also the searchindex.js script that links it all together I believe.
Exactly, everything in docs/html is generated by sphinx when you run the buildDocs.sh script. Which is also true of everything in docs/rst besides docs/rst/index.rst (hence it being added to the repo and the rest being ignored by the docs/rst/.gitignore file
It also looks like we are merging in the daystotitles.sh script however I think that the functionality of this got replaced by getTitles.py script. SO maybe we dont need to merge this?
dayToTitles.sh is the sbatch script to submit a job to summit that runs getTitles.py. There used to be a lot of logic in dayToTitles.sh that found the .day files, this logic has been moved to nav.py, making it easier to understand, update, and faster.
After some more looking it appears that the issues is that in line 269 of the dayToDF file we are using the next comand to iterate over a df.
However if the df is empty it throws the exception seen above.
I am working on handling this.
I added some error handling for this, could you try running it again and let me know how it goes?
I got a bit carried away, so this PR does a few things.