Skip to content

PR313 follow-up: proposed config and arguments format #355

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

Open
wants to merge 10 commits into
base: cosipy_pipeline
Choose a base branch
from

Conversation

israelmcmc
Copy link
Collaborator

Same functionality as #313, but I'm changing the config file and argparse arguments with a proposed general format.

… beginning to get a more meaningful error message

Signed-off-by: Israel Martinez <[email protected]>
Signed-off-by: Israel Martinez <[email protected]>
* Make it work for multiple sources
* Display the median and error for all free parameters

Signed-off-by: Israel Martinez <[email protected]>
Signed-off-by: Israel Martinez <[email protected]>
Signed-off-by: Israel Martinez <[email protected]>
@israelmcmc israelmcmc requested a review from ldigesu April 30, 2025 14:16
Copy link

codecov bot commented Apr 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.45%. Comparing base (eb587a9) to head (83e4e83).

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ldigesu
Copy link

ldigesu commented May 16, 2025

Hello Israel, I started testing this. Let me know how we proceed. Will you implement the comments and then merge in my branch, or the other way around?

In any case, for now, I noticed the following:
i) The override option does not work for me. Likely, it's me doing something wrong. E.G.The command cosi-threemlfit --config ./my_config --override “sc_file:./sc.ori” results in this error: config.override(*args.override)
File "/Users/lauradigesu/opt/anaconda3/lib/python3.9/site-packages/yayc/configurator.py", line 124, in override
key,value = args[0].split("=")
ValueError: not enough values to unpack (expected 2, got 1)

ii) I tested the time cuts, and they work for me. However, all the functions that are called there need numerical values as input, not a Time object. Thus, I made everything work by adding tstart.value/tstop.value anywhere in lines 103-108 of task.py. My working example is grb of 88s of dc3, however I'll think about how an example for this using the internal test_data of cosipy.

iii) maybe it would be useful to add the option of adding a user-defined prefix or suffix to the names of the outputs? Since everything does not get overwritten by default, maybe one wants to keep track of which source is being fitted directly in the names of the output files.

Cheers,

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