search

paulhtremblay / covid19

3 stars 4 forks source link

Improves nav #56

Closed btrem closed 4 years ago

btrem commented 4 years ago

Continues work on issues #33 and #34. Look at my comments in those issue threads while reviewing the code. Note that this branch adds module slugify to site requirements to improve urls.

paulhtremblay commented 4 years ago

no module named slugify?

btrem commented 4 years ago

pip install python-slugify It's in requirements.txt

https://github.com/paulhtremblay/covid19/commit/e7190eb9d703302ca15a92eca870a47482475699#diff-b4ef698db8ca845e5845c4618278f29a

paulhtremblay commented 4 years ago

Okay, but that is not normal to have to do that. You need to run bash test_all.sh before you do a code review: jinja2.exceptions.TemplateNotFound: site_updated_time.txt

That will show you if there are any errors. But you should also push the code to the dev bucket and then check out that everything looks ok there. That is the normal workflow.

But had to make one change in maser to get the test to work, FYI.

btrem commented 4 years ago

Okay, but that is not normal to have to do that.

You mean I shouldn't add python modules via requirements? Or I shouldn't add them at all?

You need to run bash test_all.sh before you do a code review:

bash test_all.sh generates warnings and errors, like

E.E../sweden.py:38: SettingWithCopyWarning: A value is trying to be set on a copy of a slice from a DataFrame. Try using .loc[row_indexer,col_indexer] = value instead

FileNotFoundError: [Errno 2] No such file or directory: 'html_temp/countries'

FileNotFoundError: [Errno 2] No such file or directory: 'test_data/states_population.csv'

jinja2.exceptions.TemplateNotFound: site_updated_time.txt

That will show you if there are any errors.

Well, mea culpa, obviously. I normally run bash make.sh to create the site, poke around, make sure links work, layout is correct, etc. I did that earlier in the branch, then did more changes and only checked the individual python files I was working on.

But you should also push the code to the dev bucket and then check out that everything looks ok there. That is the normal workflow.

I can't push to any bucket via my laptop. I don't have credentials. Instead I have a localhost subdomain running Apache to test the site. Not ideal, Apache !- S3, but it's better than nothing.

But had to make one change in maser to get the test to work, FYI.

You mean I need to pull master? Because this branch hasn't been merged. Right?

btrem commented 4 years ago

I have a fix for the bug, and bash make.sh does make the site correctly. But it does not pass ./tests/test_all.sh.