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

Fixes issue #35 - FITS CUNIT 'M/S' header. #36

Merged
merged 4 commits into from
May 25, 2022

Conversation

razman786
Copy link
Contributor

Fixes issue #35 - FITS CUNIT 'M/S' header.

wcslib needs 'M/S' to be lower case 'm/s' - a simple fix that updates the header.

Copy link
Member

@Athanaseus Athanaseus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ratt-priv-ci
Copy link

Can one of the admins verify this patch?

@razman786
Copy link
Contributor Author

A refined version of this PR has already been added to PR #37 for the purposes of PR 130 (ratt-ru/tigger#130).

@bennahugo
Copy link
Contributor

ok to test

@bennahugo
Copy link
Contributor

The failure stems from usage of format strings:

�[0m�[91m2022-05-23 08:40:06	INFO	imager::createFTMachine() 	Performing interferometric gridding...
�[0mlwimager normally ended
�[91mTraceback (most recent call last):
  File "/usr/local/bin/tigger-convert", line 118, in <module>
    from Tigger import Coordinates
  File "/usr/local/lib/python2.7/dist-packages/Tigger/Coordinates.py", line 371
    raise RuntimeError(f"Error WCS header {e}")
                                             ^
SyntaxError: invalid syntax
�[0m�[91mE�[0m�[91m
======================================================================
�[0m�[91mERROR: batch_test.testMeqtreesBatchJob
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/src/pyxis-1.7.1/Pyxis/recipes/meqtrees-batch-test/batch_test.py", line 112, in testMeqtreesBatchJob
    run("""tigger-convert {0:s}/test-lsm.txt --rename --format "ra_d dec_d i q u v" --center 0.1deg,60.5deg -f""".format(PACKAGE_TEST_DIR));
  File "/src/pyxis-1.7.1/Pyxis/recipes/meqtrees-batch-test/batch_test.py", line 43, in run
    raise RuntimeError("failed with exit co

Recall that I added backport facilities for Python 2.7 to fix the previous Tigger. If you want to remove these go ahead.

@razman786
Copy link
Contributor Author

@bennahugo - Ah yes I recall now (took a bit). Is anyone using tigger-lsm with Python 2.7? If not then may be the Travis test needs to be retired?

@bennahugo
Copy link
Contributor

bennahugo commented May 23, 2022

We just need to make sure that the current release only targets 3.x for those who are still on an older Tigger so we don't break the dependency tree. Then we can retire the failing build.

@bennahugo
Copy link
Contributor

Specifically this needs changing: https://github.com/ratt-ru/tigger-lsm/blob/master/setup.py#L32

@bennahugo bennahugo merged commit 14d4406 into ratt-ru:master May 25, 2022
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.

4 participants