Refactors and bugfixes #1736
Refactors and bugfixes #1736
Conversation
|
Wanting to wrap some more refactorings into here |
VeckoTheGecko
force-pushed
the
v/ref-checkpoint
branch
from
4e66000
to
17ed9bd
Compare
|
I didn't fix the #1738 into this. I wanted to do a bit of a deeper refactoring of |
| [os.remove(s) for s in [lib_file] if os.path is not None and os.path.exists(s)] | ||
| if delete_cfiles and len(all_files_array) > 0: | ||
| [os.remove(s) for s in all_files_array if os.path is not None and os.path.exists(s)] | ||
| os.remove(lib_file) | ||
| if delete_cfiles: | ||
| for s in all_files_array: | ||
| if os.path.exists(s): | ||
| os.remove(s) |
There was a problem hiding this comment.
Why don't we need to check for os.path is not None here anymore? Was that old, redundant code?
There was a problem hiding this comment.
os.path is a module of the os package, so it is never None.
A bit ago we had
from os import path
...
[... if path is not None ...]so it wasn't imediately evident before
parcels/kernel.py
Outdated
| if not g._load_chunk.flags["C_CONTIGUOUS"]: | ||
| g._load_chunk = np.array(g._load_chunk, order="C") | ||
| g._load_chunk = np.array(g._load_chunk, order="C") |
There was a problem hiding this comment.
Why is the if-statement not necessary anymore, here?
There was a problem hiding this comment.
That was a mistake. Was doing some experimenting which slipped through
parcels/tools/_helpers.py
Outdated
| @@ -56,3 +59,13 @@ def deprecated_made_private(func: Callable) -> Callable: | |||
|
|
|||
| def patch_docstring(obj: Callable, extra: str) -> None: | |||
| obj.__doc__ = f"{obj.__doc__ or ''}{extra}".strip() | |||
|
|
|||
|
|
|||
| @np.vectorize | |||
There was a problem hiding this comment.
Is it really necessary to vectorise this function? Is it really so compute-intense?
Or is this a start of more of these decorators?
There was a problem hiding this comment.
Good catch. This decorator is an easy way to make functions numpy compatible, so that you can give it an array and it can apply to each element.
I was initially using this function in TimeConverter.reltime, however since removed it from this PR. Will remove this decorator for now until it is actually needed.
Re. performance, I don't think theres any drawbacks
VeckoTheGecko
force-pushed
the
v/ref-checkpoint
branch
from
b351559
to
69b1094
Compare
for more information, see https://pre-commit.ci
timedelta_to_floathelper