Remove custom crop wrappers from GenericSpectrogram

[Amityush-lgtm]

PR Description

This PR removes all the wrapper methods that were implemented by #256 as discussed.

AI Assistance Disclosure

AI tools were used for:

• Code generation (e.g., when writing an implementation or fixing a bug)
• Test/benchmark generation
• Documentation (including examples)
• Research and understanding
• No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

[samaloney]

This is nice but I think this should be using the underlying NDCube features specifically crop_by_values and less so crop_by_values.

[Amityush-lgtm]

crop_by_values seems to be showing an error with astropy.time.Time, its showing this:

TypeError: If an element of a point is not a Quantity or None, the corresponding unit must be a valid astropy Unit or unit string.index: 0; coord type: <class 'astropy.time.core.Time'>; unit: None

Should i keep .crop() for astropy.time.Time ?

[samaloney]

Would need a bit more of an example can you give us minimal version that raise the error?

[Amityush-lgtm]

im currently using ndcube 2.4.0 and this is a demo that i made to check:

import numpy as np
import astropy.units as u
from astropy.time import Time
from radiospectra.spectrogram.spectrogrambase import GenericSpectrogram

times = Time('2021-01-01T00:00:00') + np.arange(10) * u.s
freqs = np.linspace(10, 20, 5) * u.MHz
data = np.random.rand(5, 10)
meta_dict = {
    'start_time': times[0], 
    'end_time': times[-1], 
    'times': times, 
    'freqs': freqs, 
    'wavelength': u.Quantity([freqs[0], freqs[-1]]), 
    'observatory': 'test', 
    'instrument': 'test', 
    'detector': 'test'
}
spec = GenericSpectrogram(data, meta_dict)

try:
    cropped = spec.crop_by_values((times[2], None), (times[5], None))
    print(cropped.shape)
except Exception as e:
    print(f"Failed with {type(e).__name__}: {e}")

try:
    cropped = spec.crop((times[2], None), (times[5], None))
    print(cropped.shape)
except Exception as e:
    print(f"Failed with {type(e).__name__}: {e}")

Output:
Failed with TypeError: If an element of a point is not a Quantity or None, the corresponding unit must be a valid astropy Unit or unit string.index: 0; coord type: <class 'astropy.time.core.Time'>; unit: None (5, 4)

[samaloney]

Ah yea so if you look at the WCS that is created

spec.wcs
<WCS(output_frame=CompositeFrame, input_frame=Frame-c7cd, forward_transform=Model: CompoundModel
Inputs: ('x0', 'x1')
Outputs: ('y0', 'y1')
Model set size: 1
Expression: [0] & [1]
Components:
    [0]: <Tabular1D(points=(<Quantity [0., 1., 2., 3., 4., 5., 6., 7., 8., 9.] pix>,), lookup_table=[0. 1. 2. 3. 4. 5. 6. 7. 8. 9.] s)>

    [1]: <Tabular1D(points=(<Quantity [0., 1., 2., 3., 4.] pix>,), lookup_table=[10.  12.5 15.  17.5 20. ] MHz)>
Parameters:)>

The lookup table is has units of seconds so the crop_by_values call would be spec.crop_by_values((2*u.s, None), (5*u.s, None)) and the crop call would be spec.crop((times[1], None), (times[5], None))

[Amityush-lgtm]

Ooh then i think it might not be a good idea to use crop_by_values() for time as it will not be much user friendly like currently we can pass TIme objects, time strings as well, it will work fine with crop_freq() and time_profile() . Does that sound good??

[samaloney]

I'm not sure, at this point, we should be adding extra/alternative methods but rather showing how to use the existing NDCube methods. But I do agree it not the most user friendly API at the moment

[Amityush-lgtm]

Got it. One thing I was wondering though, @hayesla had suggested making it easier to obtain things like a time profile at a specific frequency or a line profile at a specific time, would you prefer those to just be documented with examples rather than adding new methods??

[samaloney]

Yea I chatted to Laura and I think we are both say the same thing that directly slicing the spectrum e.g. spec[10:15] e.g. isn't very user friendly we should demonstrate how to extract time profile or frequency cut using .crop and .crop_by_values.

[Amityush-lgtm]

I have removed the crop_time, crop_freq, time_profile and line_profile methods, I'll now work on the examples and open a separate PR