NNPDF / reportengine

A framework for declarative data analysis
https://data.nnpdf.science/validphys-docs/guide.html
GNU General Public License v2.0
1 stars 2 forks source link

Update reportengine dependencies #61

Closed scarlehoff closed 6 months ago

scarlehoff commented 9 months ago

This PR does:

scarlehoff commented 9 months ago

I will merge this by the time https://github.com/NNPDF/nnpdf/pull/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.

RoyStegeman commented 6 months ago

@comane when you have time, could you please review this?

RoyStegeman commented 6 months ago

@scarlehoff could you add a comment explaining why you restricted the ruamel_yaml version?

scarlehoff commented 6 months ago

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...

scarlehoff commented 6 months ago

@comane are you fine with me merging this?

afaik you are the only one using the newer versions of validphys

comane commented 6 months ago

Hi @scarlehoff, I tested this branch with the most up to date version of validphys (nnpdf), by pip installing this branch. I runned some example scripts (plot pdfs and data theory comparison) and they worked well even with dask.

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.

scarlehoff commented 6 months ago

I see. I'll add bokeh then as an extra. Since dependencies of reportengine affect nnpdf as well I'd rather not add many.

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).

Yes. At least for python 3.12. But the first step is to create a conda package for this (since that's the blocker right now)