- 
                Notifications
    
You must be signed in to change notification settings  - Fork 218
 
          Deprecate tmp_parameters in favour of FileIO metadata
          #3528
        
          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
base: RELEASE_next_minor
Are you sure you want to change the base?
Conversation
tmp_parameters in favour of FileIO metadata
      
          Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@                  Coverage Diff                   @@
##           RELEASE_next_minor    #3528      +/-   ##
======================================================
+ Coverage               81.88%   81.90%   +0.02%     
======================================================
  Files                     149      149              
  Lines                   22916    22912       -4     
  Branches                 4673     4675       +2     
======================================================
+ Hits                    18764    18767       +3     
+ Misses                   2951     2945       -6     
+ Partials                 1201     1200       -1     ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
  | 
    
93549a5    to
    27f97f7      
    Compare
  
    27f97f7    to
    c82a1c6      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR deprecates the poorly documented tmp_parameters in favor of the FileIO metadata system for storing file operation information. The change improves consistency and documentation while maintaining backward compatibility through deprecation warnings.
Key Changes
- Deprecates 
tmp_parametersproperty with warning messages pointing tometadata.General.FileIO - Updates file I/O operations to store filename, folder, and extension information in 
FileIOmetadata - Refactors save/load functionality to use the new metadata structure instead of 
tmp_parameters 
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description | 
|---|---|
hyperspy/signal.py | 
Deprecates tmp_parameters property and updates save/load logic to use FileIO metadata | 
hyperspy/io.py | 
Updates file I/O operations to populate FileIO metadata instead of tmp_parameters | 
hyperspy/tests/test_io.py | 
Updates tests to use new FileIO metadata structure and removes deprecated functionality | 
hyperspy/tests/signals/test_tools.py | 
Removes deprecated tmp_parameters usage in tests | 
hyperspy/tests/drawing/test_markers.py | 
Updates test to use FileIO metadata for filename manipulation | 
doc/user_guide/io.rst | 
Updates documentation to reference FileIO metadata instead of tmp_parameters | 
doc/reference/metadata.rst | 
Adds documentation for new FileIO metadata fields | 
upcoming_changes/*.rst | 
Updates changelog entries to reflect the deprecation | 
c82a1c6    to
    eeb2d45      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code-wise this LGTM. However, there are subtle but important differences in the behaviour of the feature in respect to the previous implementation that raise issues that must be address.
tmp_parameters is not copied when e.g. slicing a signal, while metadata is. Also, tmp_parameters, unlike metadata, is not stored when saving a file. This was intentional (hence the name tmp_parameters: the file location in the file system was stored only "temporarily" because:
- It is not related to the data itself
 - Files can move
 - The transformed file (e.g. a slice) is not typically saved in a way that replaces the original file. (Of course there are methods that transform the data in-place, which is an issue also for the 
tmp_parametersimplementation. - The path contains user information, that the user may not like to store in the file
 
| ---------- | ||
| string : str | ||
| File extension, without initial "." separator | ||
| Format name or file extension, with or without initial "." separator. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not sticking only to one of the two (with our without). In HyperSpy we have typically gone for without, but in Python it is a lot more common to include the dot in the extension. Therefore, why not supporting both, with a warning that from v3.0 only the extension with dot will be supported?
| 
           Thank you @francisco-dlp for the review. I did think about these differences and thought that these can be considered as undocumented implementation details, while the functionality stay the same. What would you suggest? Not to use   | 
    
@francisco-dlp, as discussed in #3525 (comment), this is the PR to deprecate
tmp_parametersin favour of extending https://hyperspy.org/hyperspy-doc/current/reference/metadata.html#fileio.The filename, folder and extension will now be stored in https://hyperspy.org/hyperspy-doc/current/reference/metadata.html#fileio and the metadata of the last load operation will be used to define the parameters previously saved in
tmp_parameters.Corresponding rosettasciio PR: hyperspy/rosettasciio#425
Progress of the PR
zspywith directory stores - introduced in Improve file saving functionality and deprecate obsolete parameters #3525,tmp_parametersin favour ofFileIOmetadataupcoming_changesfolder (seeupcoming_changes/README.rst),readthedocsdoc build of this PR (link in github checks)