Update reportengine dependencies #61
Conversation
|
I will merge this by the time NNPDF/nnpdf#1861 is implemented. If someone (@comane I guess is the only person with experience here) can review before that so much the better. Since this is only dealing with a few test and dependencies and the latest version is already being used via conda, I guess it would be fine. |
|
@comane when you have time, could you please review this? |
| @@ -76,10 +76,13 @@ def nsspec(x, beginning=()): | |||
| self.graph.add_node(mcall, inputs={gcall, hcall}) | |||
|
|
|||
|
|
|||
| def _test_ns(self): | |||
| def _test_ns(self, promise=False): | |||
There was a problem hiding this comment.
Is there a particular reason for calling this promise?
There was a problem hiding this comment.
I think it looked like a js promise objet to me
There was a problem hiding this comment.
Hi @scarlehoff, @RoyStegeman, I simply updated the description of the PR as this is not only updating the dependencies
|
@scarlehoff could you add a comment explaining why you restricted the |
|
One of the reasons this needed review was because I did the things that made the code work. Including ruamel. This version worked, others didn't... |
| @@ -76,10 +76,13 @@ def nsspec(x, beginning=()): | |||
| self.graph.add_node(mcall, inputs={gcall, hcall}) | |||
|
|
|||
|
|
|||
| def _test_ns(self): | |||
| def _test_ns(self, promise=False): | |||
There was a problem hiding this comment.
I think it looked like a js promise objet to me
|
@comane are you fine with me merging this? afaik you are the only one using the newer versions of validphys |
| "dask[distributed]", | ||
| ] |
There was a problem hiding this comment.
| "dask[distributed]", | |
| ] | |
| "dask[distributed]", | |
| "bokeh!=3.0.*,>=2.4.2", | |
| ] |
There was a problem hiding this comment.
is bokeh a required dependency? I don't see it anywhere
|
Hi @scarlehoff, I tested this branch with the most up to date version of validphys (nnpdf), by pip installing this branch. I guess that the idea is to change the nnpdf dependency as well since right now it is checking a fixed commit (which also needs curio). I added a suggestion for bokeh. This way one gets the dask dashboard. |
|
I see. I'll add bokeh then as an extra. Since dependencies of reportengine affect nnpdf as well I'd rather not add many.
Yes. At least for python 3.12. |
This PR does:
updates the
ruamelanddaskdependenciesModify tests in
test_executor.pyandtest_builder.pyso as to be able to accept dask futures.