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

Added clean_qgis_layer decorator back #64

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

Joonalai
Copy link
Contributor

This PR:

  • Adds clean_qgis_layer decorator back since it has been requested

@Joonalai Joonalai force-pushed the clean-layers-decorator branch from 78e2fb0 to 0648005 Compare May 13, 2024 20:41
@Joonalai
Copy link
Contributor Author

Joonalai commented Jun 4, 2024

@LKajan, @ismogis could you review this?

@LKajan
Copy link
Contributor

LKajan commented Jun 10, 2024

I think this kind of approach would indeed be cleaner than figuring by the function name the need for layer cleaning.

This particular implementation seems to fail in cases where the layer fixture is using yield:

@pytest.fixture()
@clean_qgis_layer
def layer():
  layer = QgsLayer(...)
  yield layer
  print(f"teardown layer {layer}")

Could you find some alternative way for the implementation?

Have you studied what actually is causing these seg faults? Is it possible to fix the root cause instead?

@LKajan LKajan force-pushed the clean-layers-decorator branch from 0648005 to a466e4c Compare June 14, 2024 08:25
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 57.53%. Comparing base (d782092) to head (a466e4c).

Files Patch % Lines
src/pytest_qgis/utils.py 66.66% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #64      +/-   ##
==========================================
+ Coverage   57.25%   57.53%   +0.28%     
==========================================
  Files           7        7              
  Lines         496      504       +8     
==========================================
+ Hits          284      290       +6     
- Misses        212      214       +2     
Flag Coverage Δ
pytest 57.53% <66.66%> (+0.28%) ⬆️

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.

@LKajan LKajan force-pushed the clean-layers-decorator branch from a466e4c to 384d94e Compare June 14, 2024 08:39
@LKajan
Copy link
Contributor

LKajan commented Jun 14, 2024

I added a mention that this doesn't work with fixtures that yield for now and made typing more precise.
With these changes I think it is ok to merge until we find a better solution to make sure map layers don't cause seg faults.

@LKajan LKajan merged commit 3a33209 into GispoCoding:main Jun 14, 2024
4 checks passed
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.

3 participants