cxcsds / ciao-contrib

Extra scripts and code to enhance the capabilities of CIAO.
GNU General Public License v3.0
8 stars 6 forks source link

mk_runtool use p_type #780

Open kglotfelty opened 1 year ago

kglotfelty commented 1 year ago

With CIAO 4.16 we will have access to a parameter's p_type so together with using the plist method we could replace the line parsing

https://github.com/cxcsds/ciao-contrib/blob/22bbae825d73c4647d4fe5a5766b148aa8a82e66/mk_runtool.py#L145-L152

with individaul pget's to each of the p_foo properties (p_min, p_max, p_prompt, p_value, and now p_type).

DougBurke commented 1 year ago

I was wondering if this change would simplify the runtool generation, but then immediately forgot about it...

kglotfelty commented 1 year ago

Me too, that's why I created the issue so we don't forget.

I was also wondering if we can do things 'on the fly' instead of pre-generating each individual too/.par file -- but we probably have too many special cases (scripts(*), parnames that start with numbers, etc) that it's probably not worth the effort.)

(*)Though I know Joe's team is working on converting the script to python so maybe that's one less barrier.

DougBurke commented 11 months ago

Hmmm, so this is with CIAOX. When asking for p_min or p_max it is prompting me for an answer, which seems wrong (I tried paramopen with mode set to "r" and "h" and this didn't seem to help).

>>> import paramio as pio
>>> fh = pio.paramopen("dmstat")
>>> print(pio.plist(fh))
['infile', 'centroid', 'median', 'sigma', 'clip', 'nsigma', 'maxiter', 'verbose', 'out_columns', 'out_min', 'out_min_loc', 'out_max', 'out_max_loc', 'out_mean', 'out_median', 'out_sigma', 'out_sum', 'out_good', 'out_null', 'out_cnvrgd', 'out_cntrd_log', 'out_cntrd_phys', 'out_sigma_cntrd', 'mode']
>>> pio.pget(fh, "median")
'no'
>>> pio.pget(fh, "median.p_prompt")
'Calculate median value?'
>>> pio.pget(fh, "median.p_type")
'b'
>>> pio.pget(fh, "median.p_value")
'no'
>>> pio.pget(fh, "median.p_mode")
'h'
>>> pio.pget(fh, "median.p_min")
Calculate median value? (no): 
'no'
>>> pio.pget(fh, "median.p_max")
Calculate median value? (no:) (no): 
'no'

Wondering if it was because this was a boolean, I try with an integer value with min and max set and get even odder behaviour:

>>> import paramio as pio
>>> fh = pio.paramopen("dmstat")
>>> pio.pget(fh, "maxiter.p_min")
'1'
>>> pio.pget(fh, "maxiter.p_max")
Maximum number of iterations (1:) (20): 
'20'

Aha. It seems to query when the value is not set (so for maxiter we have a set minimum value but no maximum, so the query is asking for the value to use). Which is understandable but not useful here.

DougBurke commented 11 months ago

For reference, here's what happens when you have a list of options (here for merge_obs):

>>> pio.pget(fh, "psfmerge.p_min")
'exptime|expmap|min|max|mean|median|mid'

and the environment paths get expanded for you - e.g.

>>> pio.pget(fh, "tmpdir")
'/tmp'

rather than returning "${ASCDS_WORK_PATH}" (not sure if this would be an issue or not)

kglotfelty commented 11 months ago

Humm, not sure if the emails went through

if you open w/ 'rH' (upper case) then you don't get prompted ... you still get the exception if the min/max isn't set

In [3]: ff = pio.paramopen("dmstat", "rH")

In [6]: pio.pget(ff, "maxiter.p_min")
Out[6]: '1'

In [7]: pio.pget(ff, "maxiter.p_max")
/home/kjg/cxcds_param4/dmstat.par: parameter error? : maxiter.p_max
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[7], line 1
----> 1 pio.pget(ff, "maxiter.p_max")

ValueError: pget() Parameter not found

and

You can use 'p_value' to get the parameter value before it has been resolved, so

In [1]: import paramio as pio

In [2]: zz = pio.paramopen("merge_obs", "rH")

In [3]: pio.pget(zz, "tmpdir.p_value")
Out[3]: '${ASCDS_WORK_PATH}'

In [4]: pio.pget(zz, "tmpdir")
Out[4]: '/tmp'
DougBurke commented 11 months ago

Ta. Now get back to not-being-at-work!

DougBurke commented 10 months ago

So, we tried this and it didn't work for 4.16. Let's leave this open in cae we change the paramio behaviour for the next release.