Skip to content

Conversation

JesusTorrado
Copy link
Contributor

This is a duplicate of #427

cmbant and others added 11 commits June 27, 2025 14:39
- Replace clik dependency with clipy for easier installation and better performance
- Install clipy from GitHub repository to packages path instead of site-packages
- Support both CMB temperature and lensing likelihoods through clipy's conditional logic
- Update installation configuration to use planck_2018_highl_plik.TT instead of TT_unbinned
- Remove planck_2015_lowl from test installation configuration
- Use load_external_module for consistent module loading
- Simplify likelihood return value handling with float() conversion

Benefits:
- No more complex WAF compilation required
- Pure Python implementation with JAX optimization
- Backward compatibility maintained - no changes needed in user YAML files
- Supports both TT and lensing likelihoods
- Easier installation process
- Remove unused imports (re, sys)
- Remove duplicate import of load_external_module
- Fix string quotes consistency (single -> double)
- Add trailing commas and proper line breaks
- Fix exception handling style
- Improve import ordering and spacing

Applied by pre-commit hooks (ruff-check, ruff-format)
- Fix zip file handling to support both 'main.zip' and 'clipy-main.zip' filenames
- Improve directory move logic to handle cases where zip extraction already occurred
- Fix is_installed_clipy to properly check for clipy in specified packages path
- Add proper path verification to ensure clipy is loaded from correct location
- Handle both global and packages path installation scenarios

This resolves the installation verification failures and ensures clipy
is properly installed to the packages path instead of falling back to
global installation.
- Add get_clipy_import_path function to properly handle clipy directory structure
- Update load_clipy to use get_import_path for proper module loading
- Add default_global parameter to control fallback behavior
- Fix is_installed_clipy to properly check packages path vs global installation
- Ensure clipy is loaded from correct location with proper __file__ attribute

This addresses the module path verification issues and ensures clipy
is properly installed and loaded from the packages path.
- Change install method to check for clipy directory existence directly
- Remove dependency on is_installed_clipy function that was falling back to global
- Ensure clipy is always installed to packages path when not present
- Fix module loading to use proper packages path instead of global fallback

This resolves the issue where clipy was being loaded from global site-packages
instead of the intended packages path installation. Now clipy is properly
installed and loaded from the packages path as intended.
@JesusTorrado
Copy link
Contributor Author

JesusTorrado commented Sep 3, 2025

Documentation is done, and commands (https://github.com/benabed/clipy?tab=readme-ov-file#new-initialisation-time-features) are now supported.

@cmbant While I fix the errors, would you have time for a review of this PR? I think it makes sense to merge it before tacking the CLASS issues.

@cmbant
Copy link
Member

cmbant commented Sep 3, 2025

I'm on hol until 10th

@JesusTorrado
Copy link
Contributor Author

JesusTorrado commented Sep 3, 2025 via email

@cmbant
Copy link
Member

cmbant commented Sep 10, 2025

Looks fine to me, though not sure the comment

# Clipy commands -- string or list of strings, e.g. ["no TT", "only EE 217x217 500 800 lax"]

makes much sense in many of the yaml files (e.g. TE likelihood).

@JesusTorrado
Copy link
Contributor Author

Looks fine to me, though not sure the comment

# Clipy commands -- string or list of strings, e.g. ["no TT", "only EE 217x217 500 800 lax"]

makes much sense in many of the yaml files (e.g. TE likelihood).

Fair enough, but it's just an example of the syntax. I don't have the bandwidth a the moment to tailor individual ones. Let's assume users know what's meant, and leave this as a TODO for a future possible PR.

I am merging now, because the CLASS fixes need to be based on this branch. Thanks for reviewing it!

@JesusTorrado JesusTorrado merged commit 9d7031e into master Sep 12, 2025
9 of 11 checks passed
@JesusTorrado JesusTorrado mentioned this pull request Sep 17, 2025
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.

2 participants