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

Deprecate old, duplicate functions #758

Open
magnunor opened this issue Apr 13, 2021 · 4 comments
Open

Deprecate old, duplicate functions #758

magnunor opened this issue Apr 13, 2021 · 4 comments
Assignees
Milestone

Comments

@magnunor
Copy link
Collaborator

Currently there is duplication of functionality between some of the functions which came from pixstem. One example of this is Diffraction2D.shift_diffraction, which essentially does the same thing as Diffraction2D.center_direct_beam.

These duplicate functions should firstly get a DeprecationWarning, then be removed in a future release.

@magnunor magnunor added the dev label Apr 13, 2021
@pc494
Copy link
Member

pc494 commented Apr 13, 2021

It's also (as it happens) a special case of Diffraction2D.apply_affine_transformation().

Shall we use this thread as a place to keep track of such duplications?

@magnunor
Copy link
Collaborator Author

Shall we use this thread as a place to keep track of such duplications?

Sounds good!

Some other functions are radial_integration and radial_average.

I'm tempted to wait until #759 (via hyperspy/hyperspy#2703) is complete, so that the "new" functions can be implemented using s.map.

@pc494 pc494 mentioned this issue Jan 6, 2022
@CSSFrancis CSSFrancis added this to the v1.0.0 milestone Aug 17, 2022
@pc494 pc494 modified the milestones: v1.0.0, v.0.18.0 Jan 26, 2024
@pc494 pc494 self-assigned this Mar 21, 2024
@magnunor
Copy link
Collaborator Author

I've gone through the old pixstem functions in Diffraction2D, seeing what can be removed, and what should be tweaked.


I'm tempted to remove some of the the old pixstem functionality which overlaps with other functionality. For example:

  • Diffraction2D.shift_diffraction is pretty similar to center_direct_beam
  • Diffraction2D.rotate_diffraction, Diffraction2D.flip_diffraction_x and Diffraction2D.flip_diffraction_y can be replaced with two lines of code + s.map. (They're old functions from when HyperSpy's s.map functionality did not handle large datasets very well)
  • Diffraction2D.peak_position_refinement_com functionality will be moved into DiffractionVectors?
  • Diffraction2D.intensity_peaks functionality will be moved into DiffractionVectors?
  • Diffraction2D.make_probe_navigation I think this is implemented directly in HyperSpy now? With the navigator being saved in the metadata?
  • Diffraction2D.plot I think all of the functionality here is automatically handled in HyperSpy now?
  • Diffraciton2D.angular_mask this is connected to angular_slice_radial_average, which is being deprecated in 1.0.0. As the functionality is replaced with the get_azimuthal_intergral functions

Some other functions I think should be kept, but might need some tweaking?

  • Diffraction2D.find_dead_pixels I think this is fine
  • Diffraction2D.find_hot_pixels I have only ever used this for data acquired on a misconfigured detector. It is different from find_dead_pixels in that the misbehaving pixels can change for each probe position. I'm not sure how often that actually occurs
  • Diffraction2D.correct_bad_pixels this can handle the output from both find_dead_pixels and find_hot_pixels, ergo with the mishaving pixels changing at each probe position. This adds a lot of complexity to the function, and I'm not sure how well it performs nowadays.
  • Diffraction2D.threshold_and_mask I think this functionality is useful. But due to changes in how center_of_mass is handled in the DPC-restructure pull request, it needs to be updated.

  • Diffraction2D.template_match_disk preprocessing for some functions, very useful, keep

@pc494
Copy link
Member

pc494 commented Apr 30, 2024

So I (briefly) went after some of these. There is some discussion in #1019 and #1011.

I think my main feeling was that the namespace on Diffraction2D is so crowded by Signal2D that being 'clean' has a fairly poor effort/reward trade-off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants