Make it work on recent Python #53
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matteobachetti
requested review from
sergiopoppi,
giuseppe-carboni,
flyingfrog81 and
SimonaRighini
|
Not working |
|
@sergiopoppi how did you install it? |
A clean shell improved... Runbasie now works. Other problems: runbasie |
This comment was marked as resolved.
This comment was marked as resolved.
matteobachetti
changed the title
|
Current status:
When running tests, I found that they were a little unmaintained. For example, some tests were requiring old versions of configuration test files. I copied an old test file to verify everything worked, and I now only have these two failures, which I frankly don't know how to address. |
|
Just for the sake of understanding if always the same two tests don't pass, I marked them as |
matteobachetti
commented
Mar 19, 2024
| Read from beamsize_table the nearest frequency value. | ||
| If freq is None defauls to self.fmin | ||
| @param freq: frequency (MHz) | ||
| @type freq: Quantity | ||
| @return: beamsize at given frequency | ||
| """ | ||
| if not freq: | ||
| if freq is None: |
There was a problem hiding this comment.
This did not work anymore when freq was a quantity, so I explicitly used the test for None.
matteobachetti
commented
Mar 19, 2024
| @@ -10,13 +10,16 @@ | |||
| from basie import target_parser | |||
|
|
|||
| BASE_PATH = ".basie_test" | |||
| curdir = os.path.abspath(os.path.dirname(__file__)) | |||
There was a problem hiding this comment.
I changed all paths to data files to be relative to the current directory but without needing soft links (which would not work on Windows, by the way)
matteobachetti
commented
Mar 19, 2024
| @@ -0,0 +1,10 @@ | |||
| Alpha EqCross1_3 TP EQ 12:00:00h +45:00:00 | |||
There was a problem hiding this comment.
I added this file reproducing the test cases that were needed for some tests but were removed in some recent version of the example files.
matteobachetti
commented
Mar 19, 2024
| @@ -43,9 +43,15 @@ | |||
|
|
|||
| class VAngle(Angle): | |||
| def __new__(cls, angle, unit=u.deg, wrap_angle=360 * u.deg, **kwargs): | |||
| was_tuple=False | |||
| if isinstance(angle, tuple) and unit in (u.deg, u.hour): | |||
There was a problem hiding this comment.
The tuple of values does not work anymore with astropy's Angle in recent versions, due to its ambiguous behavior when the degree/hour value is 0. So, I reformatted the angle as a single number in degrees when it is given as a tuple, but kept the representation as sexagesimal so that the output does not change.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
sergiopoppi
approved these changes
Mar 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I arrived up to installing (now with
pip install .) and runningrunbasie out -c configuration_SR.txt -d.Feel free to suggest any further tests you would like to run to make sure it works!
User-facing changes:
To run tests, you can now do
pytest basie/testsor
tox -e py{38,39,310,311,312}-teststo test locally on any of those Python versionsInternal changes:
I moved around directories, eliminating soft links and giving the package a standard astropy-affiliated package structure. It seems like I changed a thousand things, but the diff is mostly the result of changing directory names.
I think it is now easier to follow where things actually are. The script
runbasieis automatically generated during installation, the version is automatically tracked from the git tags.