-
Notifications
You must be signed in to change notification settings - Fork 164
Delayed start and output tutorials for v4 #2357
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
Delayed start and output tutorials for v4 #2357
Conversation
Merge remote-tracking branch 'origin/output-time-test' into delayed-start
…nd change int64 FILL_VALUE to np.iinfo(np.int64).min to fix Parcels-code#2180
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.
Notebook looks good! Just one minor suggestions to use actual dates in the animations?
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.
Fixed, including a few remaining time output edits since decode_times was fixed
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.
Also a very nice notebooks! Some suggestions:
- "The user is ..." is a bit formal language. Change that to "Users are". More fundamentally, we could discuss whether to use second-person ("you") throughout the tutorials? What do other tutorials/documentation use?
- Make clear that adding the custom metadata needs to be done before the pset.execute()?
- A small type in "Note when when ..."
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.
Thanks for catching this, I had left it from the v3 version. I like omitting “user”, many packages seem to be using “we”, which we also do a lot already. I like it as an inclusive way of writing, as we the writers are also users explaining Parcels to ourselves and each other.
Some other packages use the second-person (“we guide you how to ...”), which I think also reads nicely, but has a bit more of a top-down tone.
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 think I like the "we" form, especially if other packages use it too. Would be good to keep that as a consistent style throughout our tutorials. Perhaps we should/could also create a short styleguide for our tutorials?
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.
Oh really? I didn't realise that. Then no need to mention it here
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.
nvm my comment about pset.execute()..
I was also considering a short styleguide for the tutorials, we can add it under the maintainer documentation section of development?
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.
Should this tutorial be in examples (instead of examples_v3?
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.
no, I have not been able to implement adding particles to the particleset during the execution, so this is just a placeholder for now
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.
In the last cell, I'm getting an error
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
Cell In[16], [line 132](vscode-notebook-cell:?execution_count=16&line=132)
128 scatter.set_offsets(np.empty((0, 2)))
131 # Create animation
--> [132](vscode-notebook-cell:?execution_count=16&line=132) anim = matplotlib.animation.FuncAnimation(fig, animate, frames=nframes, interval=100)
133 anim
File ~/Codes/ParcelsCode/.pixi/envs/test-latest/lib/python3.11/site-packages/matplotlib/_api/__init__.py:218, in caching_module_getattr.<locals>.__getattr__(name)
216 if name in props:
217 return props[name].__get__(instance)
--> [218](https://file+.vscode-resource.vscode-cdn.net/Users/erik/Codes/ParcelsCode/docs/examples/~/Codes/ParcelsCode/.pixi/envs/test-latest/lib/python3.11/site-packages/matplotlib/_api/__init__.py:218) raise AttributeError(
219 f"module {cls.__module__!r} has no attribute {name!r}")
AttributeError: module 'matplotlib' has no attribute 'animation'
|
The unit tests on mac are taking forever, is this common? |
|
Yes, we've seen this before. I think @VeckoTheGecko is working on a fix. But in the meantime, you don't need to wait for CI success on macOS; you can merge as soon as you have green ticks on the ubuntu platforms |
Have we seen this before? I don't think there is a fix to this - its just the availability of Mac runners from github. If this becomes persistent then I can ask around if others are encountering the same |
I have added the delayed start and output tutorials, which was made possible by fixes in #2333