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

fixing sweep names and making open_nexradlevel2_datatree 5x faster #240

Merged
merged 7 commits into from
Nov 13, 2024

Conversation

aladinor
Copy link
Member

@aladinor aladinor commented Nov 8, 2024

Fixing names when passing sweep to open_nexradlevel2_tree and making it 5x faster by caching the file while extracting all datasets

@aladinor aladinor changed the title fixing sweep names and making open_nexradlevel2 5x faster fixing sweep names and making open_nexradlevel2_tree 5x faster Nov 8, 2024
@aladinor aladinor changed the title fixing sweep names and making open_nexradlevel2_tree 5x faster fixing sweep names and making open_nexradlevel2_datatree 5x faster Nov 8, 2024
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

Attention: Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.07%. Comparing base (2273a48) to head (1691527).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
xradar/io/backends/nexrad_level2.py 94.87% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
- Coverage   93.52%   93.07%   -0.45%     
==========================================
  Files          26       26              
  Lines        5004     5028      +24     
==========================================
  Hits         4680     4680              
- Misses        324      348      +24     
Flag Coverage Δ
notebooktests ?
unittests 93.07% <94.87%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Collaborator

@kmuehlbauer kmuehlbauer left a comment

Choose a reason for hiding this comment

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

@aladinor Sorry, I've missed the meeting today. Great work! If you can get the Nexrad notebook to work this will be ready to go!

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@kmuehlbauer
Copy link
Collaborator

Docs failing, but this seems like a broader issue. xarray docs failing the same way.

403 Client Error: Forbidden for url: https://raw.githubusercontent.com/....

I'm thinking that either GitHub changed something on their policy or some dependency in the network stack runs havoc. If there is someone out there brave enough to hunt that one down...

@kmuehlbauer kmuehlbauer merged commit dc76619 into openradar:main Nov 13, 2024
10 of 12 checks passed
@kmuehlbauer
Copy link
Collaborator

Amazing! @aladinor 💯

@mgrover1
Copy link
Collaborator

This is awesome! Thanks @aladinor 👍

@kmuehlbauer
Copy link
Collaborator

Docs failing, but this seems like a broader issue. xarray docs failing the same way.

403 Client Error: Forbidden for url: https://raw.githubusercontent.com/....

I'm thinking that either GitHub changed something on their policy or some dependency in the network stack runs havoc. If there is someone out there brave enough to hunt that one down...

Details: readthedocs/readthedocs.org#11763

This is resolved by openradar/open-radar-data#70 and https://github.com/openradar/open-radar-data/releases/tag/v0.4.0.

A bit hacky, but only one source to change. 😁

@aladinor aladinor deleted the nexrad-sweeps branch November 16, 2024 16:46
rcjackson pushed a commit to rcjackson/xradar that referenced this pull request Nov 26, 2024
…penradar#240)

* fixing sweep names and making open_nexradlevel2 5x faster
* using NodePath to ensure 'sweep' parameter
* adding test for sweeps argument
* removing unnecesary tests
* documenting changes in history.md
* fixing sweep number
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants