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

pintk can swap inputs #1656

Merged
merged 3 commits into from
Oct 23, 2023
Merged

pintk can swap inputs #1656

merged 3 commits into from
Oct 23, 2023

Conversation

dlakaplan
Copy link
Contributor

Previously if you fed pintk the timfile and parfile in the wrong order, it would fail with an opaque message:

(pintdev) kaplan@plock[~/pythonpackages/PINT_dlakaplan] (master) % pintk tests/datafile/NGC6440E.tim tests/datafile/NGC6440E.par -vv
INFO     (pint.pintk.pulsar             ): Loading pulsar parfile: tests/datafile/NGC6440E.tim                                      
Traceback (most recent call last):
  File "/Users/kaplan/opt/anaconda3/envs/pintdev/bin/pintk", line 33, in <module>
    sys.exit(load_entry_point('pint-pulsar', 'console_scripts', 'pintk')())
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/scripts/pintk.py", line 267, in main
    app = PINTk(
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/scripts/pintk.py", line 51, in __init__
    self.openPulsar(
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/scripts/pintk.py", line 153, in openPulsar
    self.psr = Pulsar(parfile, timfile, ephem, fitter=fitter)
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/pintk/pulsar.py", line 74, in __init__
    self.prefit_model = pint.models.get_model(self.parfile)
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/models/model_builder.py", line 708, in get_model
    model = model_builder(
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/models/model_builder.py", line 187, in __call__
    self._setup_model(
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/models/model_builder.py", line 646, in _setup_model
    timing_model.validate(allow_tcb=allow_tcb)
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/models/timing_model.py", line 433, in validate
    self.validate_component_types()
  File "/Users/kaplan/pythonpackages/PINT_dlakaplan/src/pint/models/timing_model.py", line 456, in validate_component_types
    assert (
AssertionError: Model must have one and only one spindown component (Spindown or another subclass of SpindownBase).

Now it will look and if the extensions are flipped, it will swap the inputs:

(pintdev) kaplan@plock[~/pythonpackages/PINT_dlakaplan] (pintkargs) % pintk tests/datafile/NGC6440E.tim tests/datafile/NGC6440E.par -vv
DEBUG    (pint.scripts.pintk            ): Swapping inputs: parfile='tests/datafile/NGC6440E.par' and timfile='tests/datafile/NGC6440E.tim'

If the extensions otherwise don't match the expected values it will note that as log.INFO but still try to proceed. This could still be used as a clue that there is something wrong if pintk fails.

@dlakaplan dlakaplan added GUI awaiting review This PR needs someone to review it so it can be merged labels Oct 19, 2023
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (334fab7) 68.49% compared to head (88ceb69) 68.47%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1656      +/-   ##
==========================================
- Coverage   68.49%   68.47%   -0.02%     
==========================================
  Files         104      104              
  Lines       24267    24276       +9     
  Branches     4330     4333       +3     
==========================================
+ Hits        16622    16624       +2     
- Misses       6558     6565       +7     
  Partials     1087     1087              
Files Coverage Δ
src/pint/scripts/pintk.py 23.28% <22.22%> (-0.07%) ⬇️

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

@abhisrkckl
Copy link
Contributor

Is this necessary? Wouldn't it be better just to have informative error messages?

@dlakaplan
Copy link
Contributor Author

No, it's not necessary. But while writing an informative error message I realized that this would be just as easy.

@dlakaplan
Copy link
Contributor Author

@scottransom : what do you think about this? I get the input order wrong about half of the time, and this change would make it easier. But I could also just change it to a more sensible error message.

@scottransom
Copy link
Member

I like it. And I also make that mistake sometimes. Given the dynamic nature of Python, it seems like a good idea to me. And I don't see how this is likely to mess anything up.

@abhisrkckl abhisrkckl merged commit 2977542 into nanograv:master Oct 23, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review This PR needs someone to review it so it can be merged GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants