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

Add test environment without any extras installed #87

Merged
merged 11 commits into from Apr 16, 2024

Conversation

sk1p
Copy link
Member

@sk1p sk1p commented Apr 2, 2024

Updates to ensure everything outside the udf module can actually be used without installing the full LiberTEM dependency.

TODO

  • refactor code not to import libertem directly
    • in base
    • in common
    • in the tests (other than those in tests/udf/)
  • verify the fixture changes that make libertem optional

Contributor Checklist:

@sk1p sk1p added bug Something isn't working infra labels Apr 2, 2024
@sk1p sk1p added this to the 0.6 milestone Apr 2, 2024
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 87.11111% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 86.59%. Comparing base (58b58af) to head (e90dc94).

Files Patch % Lines
src/libertem_blobfinder/base/masks.py 89.75% 13 Missing and 4 partials ⚠️
src/libertem_blobfinder/base/utils.py 80.00% 5 Missing and 5 partials ⚠️
src/libertem_blobfinder/common/fullmatch.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
+ Coverage   86.56%   86.59%   +0.03%     
==========================================
  Files          12       14       +2     
  Lines         960     1179     +219     
  Branches      135      165      +30     
==========================================
+ Hits          831     1021     +190     
- Misses         85      105      +20     
- Partials       44       53       +9     

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

@sk1p
Copy link
Member Author

sk1p commented Apr 2, 2024

@uellue @matbryan52 this is the CI addition mentioned in #61 - let me know your thoughts on this, and also feel free to take over the PR

@matbryan52
Copy link
Member

@uellue @matbryan52 this is the CI addition mentioned in #61 - let me know your thoughts on this, and also feel free to take over the PR

Thanks for getting this started, I'll see if I can get it into a working state.

@matbryan52 matbryan52 self-assigned this Apr 3, 2024
@matbryan52
Copy link
Member

matbryan52 commented Apr 10, 2024

I've ported the necessary bits of code to make tox pass. The caveat at the moment is that I had to skip the doctests on the base-only testenv. Also will need to port the documentation of masks here too and solve the merge conflicts.

@matbryan52
Copy link
Member

I removed the common extras group as it no longer makes sense after these changes. If needed it could be retained as an empty group for backwards compatibility?

@sk1p
Copy link
Member Author

sk1p commented Apr 11, 2024

I removed the common extras group as it no longer makes sense after these changes. If needed it could be retained as an empty group for backwards compatibility?

I don't really think it is important to be kept, as long as we mention it in the changelog (and update the install docs)

Copy link
Member Author

@sk1p sk1p left a comment

Choose a reason for hiding this comment

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

@matbryan52 thank you for taking over!

Should the examples also be updated to replace their usage of libertem.masks / libertem.utils, to make them consistent with the rest of the code? Added a small comment inline, other than that, it looks good to me!

conftest.py Outdated Show resolved Hide resolved
matbryan52 and others added 3 commits April 15, 2024 08:33
Co-authored-by: Alexander Clausen <alex@gc-web.de>
@sk1p sk1p changed the title WIP: add test environment without any extras installed Add test environment without any extras installed Apr 16, 2024
@sk1p
Copy link
Member Author

sk1p commented Apr 16, 2024

@matbryan52 thanks for finishing this, I'll go ahead and merge!

@sk1p sk1p merged commit 13e35ed into LiberTEM:master Apr 16, 2024
19 checks passed
@sk1p sk1p deleted the add-base-tests branch April 16, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants