-
Notifications
You must be signed in to change notification settings - Fork 25
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
Refactor executors again #112
Refactor executors again #112
Conversation
There has not been a pywren release for over 3 years, since it has now become lithops. I wonder if we should remove or disable the pywren executor for this reason. |
Codecov Report
@@ Coverage Diff @@
## master #112 +/- ##
==========================================
- Coverage 96.81% 96.32% -0.49%
==========================================
Files 12 11 -1
Lines 534 490 -44
Branches 121 112 -9
==========================================
- Hits 517 472 -45
Misses 11 11
- Partials 6 7 +1
Continue to review full report at Codecov.
|
Ok so in fb8b1ee I just removed the pywren executor. If it is unmaintained, it's not feasible for us to find workarounds to keep supporting it. @tomwhite - are you using the pywren executor these days? I had a look at lithops, and it looks like it should be pretty straightforward to support. It would be nice to have a serverless option. If someone wants to implement and maintain a lithops executor, I would be happy to support that effort. |
tests/test_rechunk.py
Outdated
a_tar = dsa.from_zarr(target_array) | ||
assert dsa.equal(a_tar, 1).all().compute() | ||
a_tar = np.asarray(result) | ||
print(a_tar.shape, a_tar.dtype) |
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.
(nitpick): left a debug print here. And can / should the assert
below be uncommented?
"Programming Language :: Python :: 3.7", | ||
"Programming Language :: Python :: 3.8", | ||
"Programming Language :: Python :: 3.9", |
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.
"Programming Language :: Python :: 3.9", | |
"Programming Language :: Python :: 3.9", | |
"Programming Language :: Python :: 3.10", |
) | ||
|
||
from mypy_extensions import NamedArg |
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.
Is this a necessary runtime dependency? Might need to add it to install_requires
in the setup.py
/ setup.cfg
.
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.
LGTM, other than the need to update the deps for mypy-extensions
(if it is indeed necessary).
No, I'm not. |
For some reason the test workflow is not running. |
GitHub actions was having issues earlier, but nothing is reported at https://www.githubstatus.com/ at the moment. |
Oh, https://github.com/pangeo-data/rechunker/actions/runs/2029808478 shows
|
This was a little coding project while stuck in a plane for a few hours. Here I am basically just copying over pangeo-forge-recipes' latest executors, which we spent a lot of time optimizing.
0.4.0
#92 (I hope; have not actually tested directly yet)Locally I have pywren tests failing with a message
TypeError: cannot pickle 'weakref' object
. Was not able to debug this quickly as I'm not too familiar with pywren.