Skip to content
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

Unit tests for makers, container and component modules (part 1/X) #163

Open
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

guillaumegrolleron
Copy link
Contributor

This PR is the first part of the implementation of the unit tests for :

nectarchain.data.container
nectarchain.makers (ChargesMakers, WaveformsMakers)

It also includes modification of the core containers class to make more robust the I/O methods. Thus the pedestal workflow has been changed to fully use the Necrtachain containers (instead of the pytable module).

The LightNectarcamEventSource has been removed because you are now using the implementation of ctapipe_io_nectarcam

There are also improvement of user scripts for waveforms, charges and photostat computation

guillaume.grolleron added 30 commits June 23, 2024 15:55
ds to find computed calibration quatities
…anted one

-update pp and n values to last studies
-display wfs -> mean instead of sum
-Agg backend is not forced in Pstat calibnration
exluse user_scripts to pytest
improvement split_run method
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 69.97519% with 121 lines in your changes missing coverage. Please review.

Project coverage is 49.43%. Comparing base (b306f30) to head (6dd1e8d).
Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
...rchain/makers/calibration/gain/photostat_makers.py 12.82% 34 Missing ⚠️
...in/makers/calibration/gain/flatfield_spe_makers.py 16.66% 20 Missing ⚠️
src/nectarchain/makers/charges_makers.py 44.44% 15 Missing ⚠️
.../nectarchain/makers/component/spe/spe_algorithm.py 16.66% 10 Missing ⚠️
src/nectarchain/data/container/core.py 79.06% 9 Missing ⚠️
src/nectarchain/data/management.py 55.00% 9 Missing ⚠️
.../nectarchain/makers/calibration/pedestal_makers.py 90.47% 8 Missing ⚠️
src/nectarchain/display/display.py 0.00% 5 Missing ⚠️
...c/nectarchain/makers/extractor/charge_extractor.py 0.00% 3 Missing ⚠️
src/nectarchain/display/__init__.py 0.00% 2 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
+ Coverage   40.76%   49.43%   +8.66%     
==========================================
  Files          65       63       -2     
  Lines        4452     4566     +114     
==========================================
+ Hits         1815     2257     +442     
+ Misses       2637     2309     -328     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@guillaumegrolleron
Copy link
Contributor Author

This PR fix Issue #120

@guillaumegrolleron
Copy link
Contributor Author

And there is an error building the docs in CI only that I absolutely don't know how to fix it.
Run make -C docs html make: Entering directory '/home/runner/work/nectarchain/nectarchain/docs' /bin/sh: 1: sphinx-build: not found make: *** [Makefile:20: html] Error 127 make: Leaving directory '/home/runner/work/nectarchain/nectarchain/docs' Error: Process completed with exit code 2.

@jlenain jlenain linked an issue Jan 9, 2025 that may be closed by this pull request
@jlenain
Copy link
Collaborator

jlenain commented Jan 9, 2025

Hi @guillaumegrolleron !

Many thanks for this PR !

On the issue with the doc in CI:

And there is an error building the docs in CI only that I absolutely don't know how to fix it. Run make -C docs html make: Entering directory '/home/runner/work/nectarchain/nectarchain/docs' /bin/sh: 1: sphinx-build: not found make: *** [Makefile:20: html] Error 127 make: Leaving directory '/home/runner/work/nectarchain/nectarchain/docs' Error: Process completed with exit code 2.

This is indeed weird, the step before in the docs job should install sphinx and make the sphinx-build command available via the pip install -e .[docs]. I need to investigate.

@jlenain
Copy link
Collaborator

jlenain commented Jan 9, 2025

I also note that the build-deploy-container job is failing in CI, presumably due to a recent change in the eWaterCycle/setup-apptainer GitHub Action. Here also, I need to investigate. See:

https://github.com/cta-observatory/nectarchain/actions/runs/12686585734/job/35360086350?pr=163

@jlenain
Copy link
Collaborator

jlenain commented Jan 9, 2025

I also note that the build-deploy-container job is failing in CI, presumably due to a recent change in the eWaterCycle/setup-apptainer GitHub Action. Here also, I need to investigate. See:

https://github.com/cta-observatory/nectarchain/actions/runs/12686585734/job/35360086350?pr=163

On this point, PR #165 should fix #164 , see https://github.com/cta-observatory/nectarchain/actions/runs/12687970602/job/35363508658?pr=165

@jlenain
Copy link
Collaborator

jlenain commented Jan 9, 2025

Hi @guillaumegrolleron !

Many thanks for this PR !

On the issue with the doc in CI:

And there is an error building the docs in CI only that I absolutely don't know how to fix it. Run make -C docs html make: Entering directory '/home/runner/work/nectarchain/nectarchain/docs' /bin/sh: 1: sphinx-build: not found make: *** [Makefile:20: html] Error 127 make: Leaving directory '/home/runner/work/nectarchain/nectarchain/docs' Error: Process completed with exit code 2.

This is indeed weird, the step before in the docs job should install sphinx and make the sphinx-build command available via the pip install -e .[docs]. I need to investigate.

On this, when working on #165 , I note the doc CI passes without any issue (see https://github.com/cta-observatory/nectarchain/actions/runs/12687970615/job/35363549490?pr=165).

Is your pyproject.toml up-to-date with respect to main ?

@jlenain
Copy link
Collaborator

jlenain commented Jan 9, 2025

Hi @guillaumegrolleron !
Many thanks for this PR !
On the issue with the doc in CI:

And there is an error building the docs in CI only that I absolutely don't know how to fix it. Run make -C docs html make: Entering directory '/home/runner/work/nectarchain/nectarchain/docs' /bin/sh: 1: sphinx-build: not found make: *** [Makefile:20: html] Error 127 make: Leaving directory '/home/runner/work/nectarchain/nectarchain/docs' Error: Process completed with exit code 2.

This is indeed weird, the step before in the docs job should install sphinx and make the sphinx-build command available via the pip install -e .[docs]. I need to investigate.

On this, when working on #165 , I note the doc CI passes without any issue (see https://github.com/cta-observatory/nectarchain/actions/runs/12687970615/job/35363549490?pr=165).

Is your pyproject.toml up-to-date with respect to main ?

Actually, I get it now: in https://github.com/cta-observatory/nectarchain/pull/163/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711, you introduced the tool.pytest.ini_options section, inadvertently moving the docs dependencies out of project.optional-dependencies section. Thus the optional dependencies for the docs CI jobs are not installed on the runner, and thus sphinx-build is not installed.

@guillaumegrolleron
Copy link
Contributor Author

Hi @guillaumegrolleron !
Many thanks for this PR !
On the issue with the doc in CI:

And there is an error building the docs in CI only that I absolutely don't know how to fix it. Run make -C docs html make: Entering directory '/home/runner/work/nectarchain/nectarchain/docs' /bin/sh: 1: sphinx-build: not found make: *** [Makefile:20: html] Error 127 make: Leaving directory '/home/runner/work/nectarchain/nectarchain/docs' Error: Process completed with exit code 2.

This is indeed weird, the step before in the docs job should install sphinx and make the sphinx-build command available via the pip install -e .[docs]. I need to investigate.

On this, when working on #165 , I note the doc CI passes without any issue (see https://github.com/cta-observatory/nectarchain/actions/runs/12687970615/job/35363549490?pr=165).
Is your pyproject.toml up-to-date with respect to main ?

Actually, I get it now: in https://github.com/cta-observatory/nectarchain/pull/163/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711, you introduced the tool.pytest.ini_options section, inadvertently moving the docs dependencies out of project.optional-dependencies section. Thus the optional dependencies for the docs CI jobs are not installed on the runner, and thus sphinx-build is not installed.

Oh yes I see, thanks a lot ! Sorry I fix it and all should be good

pyproject.toml Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop LightNectarCAMEventSource from nectarchain
2 participants