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

Frontend: Deprecate OFP and purge from test base #411

Merged
merged 5 commits into from
Oct 18, 2024
Merged

Conversation

mlange05
Copy link
Collaborator

@mlange05 mlange05 commented Oct 17, 2024

Note: Just testing for now...

As described in #209 , this PR deprecates the OFP frontend and adds a warning to the central OFP parse method. It also purges the OFP symbol from the outward-facing API and the test base (including the available_frontends() utiltiy), leaving internal mechanics to rely on the explicit mention of Frontend.OFP.

Please note that the frontend itself is still present and functional, but it's use is now discouraged.

@mlange05 mlange05 requested a review from reuterbal October 17, 2024 18:31
The effectively remove all parametrised test from the test base.
This should make it unavailable to all tests, but keep the basic
functionality intact.
Copy link

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/411/index.html

Copy link

codecov bot commented Oct 17, 2024

Codecov Report

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

Project coverage is 93.04%. Comparing base (cc30c9e) to head (f6c2151).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
loki/frontend/ofp.py 66.66% 1 Missing ⚠️
loki/transformations/tests/test_pool_allocator.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #411      +/-   ##
==========================================
- Coverage   95.96%   93.04%   -2.92%     
==========================================
  Files         198      198              
  Lines       39320    39320              
==========================================
- Hits        37734    36587    -1147     
- Misses       1586     2733    +1147     
Flag Coverage Δ
lint_rules 96.39% <ø> (ø)
loki 93.00% <96.72%> (-2.96%) ⬇️

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.

@mlange05 mlange05 marked this pull request as ready for review October 17, 2024 19:20
@mlange05
Copy link
Collaborator Author

Tests all passing, but coverage is decreased (as expected). Farewell old friend. 🫡

Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

Thank you very much, this is fantastic and sad and soothening and so many mixed feelings...

As discussed offline, I have captured the indirect coverage regressions that are incurred from this in #406 (comment)

These should be taken care of when the factual removal of OFP is happening

@reuterbal reuterbal added the ready for merge This PR has been approved and is ready to be merged label Oct 18, 2024
@reuterbal reuterbal merged commit 58adb24 into main Oct 18, 2024
12 of 13 checks passed
@reuterbal reuterbal deleted the naml-deprecate-ofp branch October 18, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for merge This PR has been approved and is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants