hyperspy / rosettasciio

Python library for reading and writing scientific data format
https://hyperspy.org/rosettasciio
GNU General Public License v3.0
44 stars 26 forks source link

File_Writer for DigitalSurf files #280

Closed Attolight-NTappy closed 2 weeks ago

Attolight-NTappy commented 1 month ago

Description of the change

This PR implements save support for digitalsurf file formats:

Progress of the PR

Minimal example of the bug fix or the new feature

import numpy as np
from rsciio.digitalsurf import file_writer,file_reader
md = { 'General': {},
       'Signal': {}}

ax  = {'name': 'X',
 'navigate': False,
 }

sd = {"data": np.arange(24)+111,
       "axes": [ax],
       "metadata": md,
       "original_metadata": {}}

file_writer("test.pro",sd)
file_reader('test.pro')[0]['data']
#array([111., 112., 113., 114., 115., 116., 117., 118., ... 133., 134.])
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 97.51773% with 14 lines in your changes missing coverage. Please review.

Project coverage is 87.66%. Comparing base (e921e77) to head (1f5e4c5). Report is 32 commits behind head on main.

Files Patch % Lines
rsciio/digitalsurf/_api.py 97.50% 0 Missing and 14 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #280 +/- ## ========================================== + Coverage 86.33% 87.66% +1.33% ========================================== Files 83 83 Lines 10672 11150 +478 Branches 2331 2414 +83 ========================================== + Hits 9214 9775 +561 + Misses 939 860 -79 + Partials 519 515 -4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Attolight-NTappy commented 3 weeks ago

This is now ready for review :)

ericpre commented 3 weeks ago

pre-commit.ci autofix

Attolight-NTappy commented 3 weeks ago

Thanks for review!

I made a few comments and maybe it would be good to increase the coverage? There are a couple of if block which are not coverage.

Alright, wait for it I'll push some new tests later today

ericpre commented 2 weeks ago

parse_metadata needs to be part of the public API, this is the test that needs to be updated to take into account this change.

Attolight-NTappy commented 2 weeks ago

Ok. I thought you would not consider changing this. Hadn't realized some other modules where already deviating from the file_reader,file_writer norm my bad

Attolight-NTappy commented 2 weeks ago

Ok finally I think we're there.

ericpre commented 2 weeks ago

Thank you @Attolight-NTappy, I push some tweaks to please ruff and improve the release note.

Attolight-NTappy commented 2 weeks ago

🤩