calum-chamberlain / ESCI451-Python

Introduction to Python for VUW ESCI 451 course.
GNU General Public License v3.0
11 stars 5 forks source link

2024 updates #33

Closed erhmestel closed 7 months ago

erhmestel commented 7 months ago

Initial set of 2024 updates for testing

review-notebook-app[bot] commented 7 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

calum-chamberlain commented 7 months ago

Can you update the environment.yml file please El to the one you were using (e.g. update the Python version, add nodefaults to channels and unpin pygmt).

calum-chamberlain commented 7 months ago

Note: to run tests locally you can run: pytest --nbval --sanitize-with doc_sanitize.cfg

Before running the tests I would recommend running all the cells to make sure you get the results you expect: jupyter nbconvert --execute --to notebook --inplace your-notebook.ipynb

calum-chamberlain commented 7 months ago

Hey El, I'm just playing with the pinning and updating the test runner to python 3.11. I will push some changes here shortly. C

calum-chamberlain commented 7 months ago

Okay, looks like tests are running. The test fails in module 3A look like they could be worked around by setting an enddate to the request to get earthquakes.

The fail in 1B looks like the first cell wasn't run before putting it up here.

In the pygmt plotting, the colour scheme for depth isn't looking great - you might try either limiting the maximum depth of earthquakes requested, and/or inverting the colourscheme.

erhmestel commented 7 months ago

Thanks, Calum.

The other issue we have: unable to upload 3B because the file once run is too large (~40MB). Git is blocking anything greater than 25MB. I can upload it without the results, but that messes up the testing. Any suggestions?

First cell in 1B is the 100 / 0 deliberate error cell. The error meant the tests I ran just failed and stopped! Earthquakes in 3A now limited 2015-01-01 to 2020-01-01 with an added note about the end date meaning we get EQs to end of 2019. Colourscheme reversed direction with the updated pygmt version - fixed this yesterday, not pushed it yet!

Will push all these as a group once fixed!

calum-chamberlain commented 7 months ago

Module 3B:

It looks like the final massive seaborn figure is the issue there. Try adding the line:

sns.set_theme(rc={"figure.dpi": 96})

in cell 33 before making the big PairGrid plot. This reduced the size of the notebook dramatically for me. You could add a comment next to that line of code explaining what it does and that they can change it to higher numbers if they want.

There are also lots of warnings in 3B with the environment that I pushed about date_parser being depreciated. If you are getting them as well then please correct that - the fewer warnings the better as the students often get hung up thinking they are errors (some are okay because it gets them used to differentiating between errors and warnings).

3A errors look to still be related to no enddate?

erhmestel commented 7 months ago

Thanks, Calum.

Yep, date_parser is depreciated, replaced by date_format. I have swapped that over in the 3B notebook I just pushed. This also meant removing your parser function and replacing just with the date format string assigned to variable parser.

I think the only warnings I am getting now are the: import pandas as pd warning about pyarrow becoming a required dependency. Pretty sure that one is okay!

3A errors - there was a second use to the function that I missed - correcting it now.

Agree it is definitely that big plot at the end - the dpi change is not working at the moment but I will persevere.

Also, going to give Aleks your more lightly pinned yml and ask him to put it on the 501 computers and the sgess001 server.