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

Sparse2D version 3 #183

Merged
merged 15 commits into from
Jan 19, 2023
Merged

Sparse2D version 3 #183

merged 15 commits into from
Jan 19, 2023

Conversation

sfarrens
Copy link
Contributor

@sfarrens sfarrens commented Dec 14, 2022

Summary

  • Updated Sparse2D version to 3.0.0, which now handles building of Python bindings and supports Apple Silicon (M1/M2 chips)
  • Removed redundant binding files
  • Deprecated use of PyQT, pyqtgraph needs to be installed manually otherwise show methods will raise an error, methods to be removed in follow-up release
  • Updated default Python version in Conda environment to 3.10
  • Switched testing framework to PyTest and dropped testing for Python 3.6
  • Added develop.txt with development dependencies
  • Other minor clean up changes

Issues

  • The latest version of Sparse2D breaks the MR 2D1D tests, these have been skipped and an issue has been opened to resolve this in a future patch
  • There is also an issue with mr_deconv in the latest version of Sparse2D, which raises a segementation fault in some cases, the corresponding test has been skipped and an issue has been opened to investigate

@sfarrens sfarrens added enhancement clean up small corrections and improvements labels Dec 14, 2022
@sfarrens sfarrens self-assigned this Dec 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 18, 2023

Codecov Report

Merging #183 (d3a0781) into develop (ae34ae4) will increase coverage by 1.57%.
The diff coverage is 45.33%.

@@             Coverage Diff             @@
##           develop     #183      +/-   ##
===========================================
+ Coverage    61.88%   63.46%   +1.57%     
===========================================
  Files           30       36       +6     
  Lines         1658     2184     +526     
===========================================
+ Hits          1026     1386     +360     
- Misses         632      798     +166     
Flag Coverage Δ
unittests 63.46% <45.33%> (+1.57%) ⬆️

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

Impacted Files Coverage Δ
examples/pysap/structure.py 100.00% <ø> (ø)
pysap/base/loaders/mat.py 50.00% <ø> (ø)
pysap/base/observable.py 45.00% <ø> (ø)
pysap/base/plugins.py 0.00% <0.00%> (-37.15%) ⬇️
pysap/base/utils.py 35.00% <ø> (ø)
pysap/configure.py 27.77% <0.00%> (ø)
pysap/data.py 68.84% <0.00%> (-0.28%) ⬇️
pysap/extensions/formating.py 19.63% <0.00%> (ø)
pysap/extensions/sparse2d.py 100.00% <ø> (ø)
pysap/extensions/tools.py 52.20% <ø> (-0.74%) ⬇️
... and 23 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sfarrens sfarrens marked this pull request as ready for review January 18, 2023 17:46
@sfarrens sfarrens requested a review from paquiteau January 18, 2023 17:58
Copy link
Contributor

@paquiteau paquiteau left a comment

Choose a reason for hiding this comment

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

Great work, this put PySAP on better tracks for its installation and usage.

As a more general reflexion I think this PR could also be the occasion to move to a standart pyproject.toml file (which will combine develop.txt, requirements.txt, setup.cfg etc...)

For the CI i recommend to switch to the more general setup-python 1 action, to be more close to the user experience.

Footnotes

  1. as recommended by github: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python

Copy link
Contributor

@paquiteau paquiteau left a comment

Choose a reason for hiding this comment

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

LGTM !

@paquiteau paquiteau closed this Jan 19, 2023
@paquiteau paquiteau reopened this Jan 19, 2023
@sfarrens
Copy link
Contributor Author

Thanks a lot @paquiteau! 🙏

I will need to update the git tag for Sparse2D once I make release over there, but I can do that in a separate PR, before we make a new PySAP release.

@sfarrens sfarrens merged commit f5abaf3 into CEA-COSMIC:develop Jan 19, 2023
@sfarrens sfarrens deleted the sparse2d_v3 branch January 19, 2023 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean up small corrections and improvements enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants