MAINT: cleanup rendering warnings #321

bsipocz · 2024-08-27T02:06:23Z

This PR cleans up 35 warnings we currently see. There are only two untouched, one is in conflict/will be fixed in #316 while the other is related to the cloud access clean-ups and #311
(there maybe some sphinx deprecations, too, I have opened upstream PRs to fix those, we cannot do much about them here (other than adding a sphinx pin, but given we don't yet fail the CI on warnings, I don't think we gain anything by doing it)

The PR is put on top of #318 and should be rebased once that is merged in.

troyraen

I tried running most of these on Fornax and ran into a bunch of issues. Some are probably unrelated to this PR. Issues that do seem to be related to this PR are:

I have to manually select the kernel after opening the notebooks.
Changing code cells starting with ! to bash cells causes them to be rendered as markdown cells and prevents them from executing.

I did not try to run the spectroscopy notebooks since the changes here are only to text, not code.

troyraen · 2024-08-27T18:38:17Z

forced_photometry/multiband_photometry.md

I ran into several problems with this notebook:

On Fornax I had to manually select a kernel for this notebook. I think it should be set to 'science_demo' since that's apparently the only one with tractor installed.

(This looks like nway.py call is not general #313 and probably unrelated to this PR.) The cell

# call nway !/home/jovyan/.local/bin/nway.py 'data/Chandra/COSMOS_chandra.fits' :ERROR_RADIUS 'data/multiband_phot.fits' 0.1 --out=data/Chandra/chandra_multiband.fits --radius 15 --prior-completeness 0.9 #!/opt/conda/bin/nway.py 'data/Chandra/COSMOS_chandra.fits' :ERROR_RADIUS 'data/multiband_phot.fits' 0.1 --out=data/Chandra/chandra_multiband.fits --radius 15 --prior-completeness 0.9

fails for me with /bin/bash: line 1: /home/jovyan/.local/bin/nway.py: No such file or directory. Using the commented-out line that points to /opt/conda/... instead of /home/jovyan/... works.

The first figure in section '5. Plot Final Results' is empty and the second one throws an AttributeError. I don't know whether it's related to this PR.

the nway cell is being fixed by #316; I just have one minor question there for which I would like to see other opinions. (my preference would be to do the PATH change up at the very top where we also add code_src, but if I'm alone with this preference, then it can go in as-is)

troyraen · 2024-08-27T20:59:09Z

light_curves/ML_AGNzoo.md

-  name: conda-root-py
+  name: python3


With this change, I have to manually select the kernel. Changing this back to name: conda-root-py works out of the box.

those custom kernel names doesn't work for the CI. I would recommend opening a separate bug report issue for it, though #303 may cover it.

troyraen · 2024-08-27T21:49:35Z

light_curves/ML_AGNzoo.md

-```{code-cell} ipython3
+```bash
 # To download the data file containing the light curves from Googledrive
-!gdown 1gb2vWn0V2unstElGTTrHIIWIftHbXJvz -O ./data/df_lc_020724.parquet.gzip
+$ gdown 1gb2vWn0V2unstElGTTrHIIWIftHbXJvz -O ./data/df_lc_020724.parquet.gzip
 ```


With this change, the bash code becomes part of the markdown cell above and is not executable.

troyraen · 2024-08-27T22:23:38Z

light_curves/requirements_ML_AGNzoo.txt

Starting with a clean environment, I had to make a few additions to this file.

troyraen · 2024-08-27T22:24:00Z

light_curves/requirements_ML_AGNzoo.txt

+# beginning of the notebook, make sure the lists are consistent and only
+# contain dependencies that are actually used in the notebook.
+tqdm
+numpy


Suggested change

numpy

numpy<2.0 # sompy requires numpy<2.0

Yeap, it's done here in fada9e7

troyraen · 2024-08-27T22:24:42Z

light_curves/requirements_ML_AGNzoo.txt

+numpy
+scipy
+pandas
+matplotlib


Suggested change

matplotlib

matplotlib

scikit-image # sompy requires scikit-image

I missed this one, thanks! will rather add it to fada9e7

troyraen · 2024-08-27T22:34:12Z

light_curves/scale_up.md

-```{code-cell}
-!cat output/lightcurves-demo-SDSS-500k/logs/scale_up.sh.log
+```bash
+$ cat output/lightcurves-demo-SDSS-500k/logs/scale_up.sh.log


With this change, the bash code becomes part of the markdown cell above and is not executable. Same for the four other changes in this file.

troyraen · 2024-08-27T22:42:10Z

light_curves/requirements_light_curve_classifier.txt

+# List explicit python dependencies here. They are also spelt out at the
+# beginning of the notebook, make sure the lists are consistent and only
+# contain dependencies that are actually used in the notebook.
+numpy


In the science_demo environment, I had to also install numba.

Suggested change

numpy

numba # required by Arsenal (sktime)

numpy

It gets pulled without issues both locally and in CI, so I would rather not list it as we don't use numba directly.
(see the log in #309)

troyraen · 2024-08-27T22:56:18Z

light_curves/light_curve_generator.md

tqdm doesn't seem to be working anymore. The iterator doesn't progress and the time stays at "00:00<?". My guess is that it's from #300. Here's an example screenshot:

Hmm, interesting. Do you have this issue in the other notebooks, too? Do you have ipywidgets installed?

(as it's already in main, could you open a bug report issue for it?)

Do you have this issue in the other notebooks, too?

Yes, in ML_AGNzoo at least.

Do you have ipywidgets installed?

Yes, ipywidgets==8.1.5

Opened #323

troyraen · 2024-08-27T23:04:58Z

light_curves/ML_AGNzoo.md

I've had this notebook running for about an hour and it's still not done. It has been stuck on the second cell in section '4) Repeating the above, this time with ZTF + WISE manifold' for most of the time. It hasn't crashed (top shows that the CPU is still in use), though there are a bunch of warnings. I don't know whether this is normal/expected or not.

I can't comment on that as I got stuck with the data download both locally and in CI, maybe open a separate issue for it.

Yes, I can confirm that I also timeout on that cell locally.

Opened #324

troyraen · 2024-08-27T23:11:53Z

light_curves/light_curve_classifier.md

-  name: conda-env-science_demo-py
+  name: python3


With this change, I have to manually select the kernel. Changing this back to name: conda-env-science_demo-py works out of the box.

bsipocz · 2024-08-28T00:04:40Z

OK, so I'll revert the last commit (7904008) that changed the ! bash command cells to a shell cell as while the change fixes the warnings it's also breaks those cells (e.g. #322)

bsipocz · 2024-08-28T17:18:19Z

OK, so given some of the issues we run into here, I would reorganise the commits in this and #309, and bring the commits into this that are uncontroversial (additional dependency installs, etc.) at the expense of knowingly not fixing all the warnings.

And all the rest that are needed for execution but causing some inconveniences (e.g. kernel names) will be gathered in the actual CI PR.

…nd neither add a line break without an empty line in the list)

bsipocz · 2024-09-12T21:18:35Z

OK, so I cleaned up this PR and only included the commits (and new fixes) that didn't cause any issues above. The rest will go into #309

MAINT: cleanup rendering warnings 6d66e51

bsipocz added the maintanence label Aug 27, 2024

bsipocz requested a review from troyraen August 27, 2024 02:08

bsipocz mentioned this pull request Aug 27, 2024

CI: execute for rendering #309

Merged

6 tasks

troyraen reviewed Aug 27, 2024

View reviewed changes

troyraen mentioned this pull request Aug 27, 2024

MAINT: Uniformise how to handle dependency install cell #318

Merged

bsipocz force-pushed the MAINT_warning_cleanups branch from 7904008 to 91b88d6 Compare August 28, 2024 00:08

This was referenced Aug 28, 2024

BUG: tqdm progress bar doesn't progress #323

Open

ML_AGNzoo doesn't finish executing #324

Closed

bsipocz added 9 commits September 12, 2024 14:00

MAINT: Be explicit, cleanup not in toctree warnings

4f1f4b2

MAINT: cleanup transitions

d0978ce

MAINT: update year

d2cd0d5

MAINT: make author names consistent

f7e01a8

MAINT: fix heading level

52ae1bc

MAINT: adding optional dependency of a direct dependency we use

c643cc9

CI: being more explicit about versions and extra dependencies

3d56d51

MAINT: making config more robust for local runs

478f775

MAINT: fix bullet points for rendering (the HTML ones don’t render, a…

b104f04

…nd neither add a line break without an empty line in the list)

bsipocz force-pushed the MAINT_warning_cleanups branch from 91b88d6 to b104f04 Compare September 12, 2024 21:09

bsipocz merged commit 6d66e51 into nasa-fornax:main Sep 12, 2024
3 checks passed

bsipocz deleted the MAINT_warning_cleanups branch September 12, 2024 21:19

github-actions bot pushed a commit that referenced this pull request Sep 12, 2024

Merge pull request #321 from bsipocz/MAINT_warning_cleanups

2778c29

MAINT: cleanup rendering warnings 6d66e51

github-actions bot pushed a commit to bsipocz/fornax-demo-notebooks that referenced this pull request Sep 12, 2024

Merge pull request nasa-fornax#321 from bsipocz/MAINT_warning_cleanups

227c054

MAINT: cleanup rendering warnings 6d66e51

This was referenced Sep 13, 2024

Reproducible image builds for Fornax images nasa-fornax/fornax-images#10

Open

BUG: kernelspec metadata incompatibility between FSC and CI/local #331

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: cleanup rendering warnings #321

MAINT: cleanup rendering warnings #321

bsipocz commented Aug 27, 2024

troyraen left a comment

troyraen Aug 27, 2024

bsipocz Aug 28, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024

troyraen Aug 27, 2024

troyraen Aug 27, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024

troyraen Aug 27, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024

bsipocz Aug 27, 2024

troyraen Aug 28, 2024

troyraen Aug 28, 2024

troyraen Aug 27, 2024

bsipocz Aug 27, 2024 •

edited

Loading

bsipocz Aug 28, 2024

troyraen Aug 28, 2024

troyraen Aug 27, 2024

bsipocz commented Aug 28, 2024

bsipocz commented Aug 28, 2024

bsipocz commented Sep 12, 2024

	matplotlib
	matplotlib
	scikit-image # sompy requires scikit-image

MAINT: cleanup rendering warnings #321

MAINT: cleanup rendering warnings #321

Conversation

bsipocz commented Aug 27, 2024

troyraen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsipocz commented Aug 28, 2024

bsipocz commented Aug 28, 2024

bsipocz commented Sep 12, 2024

bsipocz Aug 27, 2024 •

edited

Loading