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.

Sign up for GitHub

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

Jump to bottom

New release & multi-frequency handling #311

Merged
merged 67 commits into from
Feb 8, 2022
Merged

New release & multi-frequency handling #311

merged 67 commits into from
Feb 8, 2022

Conversation

ddobie
Copy link
Collaborator

@ddobie ddobie commented Jan 12, 2022 •
edited
Loading

Fix #106.
Fix #206.
Fix #280.
Fix #297.
Fix #301.
Fix #306.

ddobie and others added 2 commits January 12, 2022 22:39
@ddobie
Updated data file naming scheme
32b355d
@ddobie
Implement read_selavy function and update code/tests accordingly.
e7ef6e5 
* Update utils.py

* Implement read_selavy

* Update existing tests

* Add read_selavy testing

* Fixed _get_selavy_path

* Add _get_selavy_path testing

* Updated add_files testing
ddobie and others added 6 commits January 14, 2022 12:28
@ddobie
Added multi-frequency handling
715ed95
@ddobie
Query all epochs by default + correctly handle RACS
683cd7c 
* Update find_sources to automatically query all epochs

* Remove epoch 14 from all-vast epochs

* Correctly handle RACS check

* minor fixes

* Add RACS_EPOCHS variable

* PEP8

* Updated changelog
@ddobie
Replaced null values in racs_low_info.csv
b1298b1
@ddobie
Updated changelog
b5a383e
@ddobie
Updated frequency columns in racs info
86c1292
@ddobie
Updated frequency in test data
d819f7a
@ddobie ddobie requested a review from ajstewart January 17, 2022 09:39
@ddobie
Copy link
Collaborator Author

ddobie commented Jan 17, 2022

@ajstewart the tests and docs aren't complete yet, but can you take a look at the code itself to check you're okay with it before I proceed further?

@ddobie ddobie marked this pull request as ready for review January 17, 2022 23:04
ajstewart
ajstewart reviewed Jan 18, 2022
View reviewed changes
Copy link
Collaborator

@ajstewart ajstewart left a comment

Choose a reason for hiding this comment

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

Changes and functionality look good to me so far. I have some comments and questions but the main one is about the handling of how it knows which field centre to fetch for which band - see comment on survey.py.

CHANGELOG.md Outdated Show resolved Hide resolved
vasttools/__init__.py Show resolved Hide resolved
vasttools/bin/find_sources.py Outdated Show resolved Hide resolved
vasttools/query.py Show resolved Hide resolved
vasttools/query.py Outdated Show resolved Hide resolved
vasttools/query.py Outdated Show resolved Hide resolved
vasttools/source.py Outdated Show resolved Hide resolved
vasttools/survey.py Show resolved Hide resolved
vasttools/survey.py Show resolved Hide resolved
vasttools/tools.py Outdated Show resolved Hide resolved
@vast-bot
Copy link

vast-bot commented Jan 19, 2022 •
edited
Loading

Hello @ddobie! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-02-08 00:04:08 UTC

@askap-vast askap-vast deleted a comment from pep8speaks Feb 1, 2022
@ddobie ddobie requested a review from ajstewart February 1, 2022 02:50
ajstewart
ajstewart reviewed Feb 7, 2022
View reviewed changes
Copy link
Collaborator

@ajstewart ajstewart left a comment

Choose a reason for hiding this comment

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

The functionality and tests of them look good to me!

I only have some general tidy/pedantic (sorry) comments and suggestions.

CHANGELOG.md Outdated Show resolved Hide resolved
tests/data/psr-j2129-04-pipe-meas.csv Outdated Show resolved Hide resolved
tests/test_init.py Outdated Show resolved Hide resolved
vasttools/query.py Outdated Show resolved Hide resolved
vasttools/query.py Show resolved Hide resolved
vasttools/tools.py Outdated Show resolved Hide resolved
vasttools/utils.py Outdated Show resolved Hide resolved
vasttools/utils.py Outdated Show resolved Hide resolved
vasttools/survey.py Outdated Show resolved Hide resolved
tests/test_survey.py Outdated Show resolved Hide resolved
ddobie and others added 12 commits February 8, 2022 09:33
@ddobie @ajstewart
Update CHANGELOG.md
251f254 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update tests/test_init.py
204c1cd 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update vasttools/query.py
4324328 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update vasttools/survey.py
2929fd3 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update vasttools/utils.py
295d4d5 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update tests/test_survey.py
0683923 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie @ajstewart
Update vasttools/tools.py
556a98b 
Co-authored-by: Adam Stewart <ajstewart@users.noreply.github.com>
@ddobie
Remove epoch 99 comment
31138ad
@ddobie
Add meta comment
c0b2040
@ddobie
PEP8
2ccb90f
@ddobie
utils update
7510fc9
@ddobie
Merge conflicts
27945be
@pep8speaks
Copy link

pep8speaks commented Feb 7, 2022 •
edited
Loading

Hello @ddobie! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-02-08 00:04:08 UTC

@ddobie ddobie requested a review from ajstewart February 8, 2022 00:07
@ddobie ddobie merged commit eaddc61 into dev Feb 8, 2022
@ajstewart ajstewart deleted the v3.0.0-draft branch February 8, 2022 12:45
@ddobie ddobie restored the v3.0.0-draft branch March 17, 2023 00:18
@ddobie ddobie deleted the v3.0.0-draft branch August 25, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajstewart ajstewart ajstewart approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@ddobie @vast-bot @pep8speaks @ajstewart