-
Notifications
You must be signed in to change notification settings - Fork 66
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
Fix colormap and climatology calculations SeaIceSeasonality_DFA.ipynb notebook #288
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
This pull request has been mentioned on ACCESS Hive Community Forum. There might be relevant details there: https://forum.access-hive.org.au/t/cosima-hackathon-v3-0-monday-september-4th-2023/1186/4 |
@anton-seaice could you review this PR? |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:30Z Please update the conda/analysis version. I don't think this comment about the drop down list makes sense anymore |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:31Z Line #2. #exp = "01deg_jra55v140_iaf_cycle2" Please remote this line and fix the comment in line one to refer to the right model run. |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:32Z Line #88. time = pd.date_range(str(year)+'-02-15', str(year)+'-02-16', freq = 'D', closed = 'left') replace |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:32Z Line #77. cbar_ticks = [int(i) for i in np.arange(-150, 151, 50)] cbar_ticks = np.arange(-150, 151, 50) |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:33Z Line #82. cbar_ticks = [int(i) for i in levels] cbar_ticks = levels |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:34Z Line #87. cbar_ticks = [int(i) for i in np.arange(5, timesteps+1, 20)] cbar_ticks = np.arange(0, timesteps+1, 20) |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:35Z Line #2. dir_out = '.' giving these two variables clear names would be good. And using ALLCAPS or start with an _ to show they are constants
I would just define them at the start of the file I think |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:36Z Line #5. for i in np.arange(0, len(stime)): We really should use a groupby_bins instead of this loop, but I realise that's not in scope for the review! |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:37Z Line #2. filein = '.' We may as well use the same name and variable as the cell above? |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:37Z Line #4. adv_list = glob.glob(dir_out + "*Adv*.nc") Remove unneeded line 4/5/6 for consistency with how the rest of the notebook works |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:38Z Line #7. adv_list, ret_list, dur_list = getFileList(filein, np.arange(start_year, end_year)) start_year and end_year are not defined at this point |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:39Z Line #1. #File path of folder containing netcdf files delete old comment |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:40Z Line #5. yrs = np.arange(start_year, end_year) I dont see how this line works, you deleted start_year and end_year earlier |
View / edit / reply to this conversation on ReviewNB anton-seaice commented on 2023-10-02T01:00:41Z I'm not sure why the first output plot is blank? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments in the reviewNB. I find all the saving and retrieving from extra files super confusing and think the whole notebook should be simplified. I guess it just depends how keen we are to do that now?
I'm closing this as I feel it's become stale; sorry. |
closes #271