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

Store antenna_by_depth in detector json #792

Open
sjoerd-bouma opened this issue Jan 17, 2025 · 2 comments
Open

Store antenna_by_depth in detector json #792

sjoerd-bouma opened this issue Jan 17, 2025 · 2 comments
Labels
enhancement New feature or request

Comments

@sjoerd-bouma
Copy link
Collaborator

The Detector class on initialization has the annoying default argument antenna_by_depth=True even though this results in unusable antenna names for most detectors I have seen in the past 4 years. While we could change this and probably save a lot of inexperienced users some headaches at the cost of technically breaking backwards-compatibility, whether or not antennas are stored as full antenna descriptions or as short strings where the antenna depth suffix should be automatically added is a property of the json, not something that is dynamic at class initialization. I therefore think we should move towards just storing this value inside the detector jsons.

This should be a relatively straightforward change.

@sjoerd-bouma sjoerd-bouma added the enhancement New feature or request label Jan 17, 2025
@fschlueter
Copy link
Collaborator

I absolutely agree. I think it caused me headaches twice (because I forgot it after the first time). Would be great to avoid it in the future!

@cg-laser
Copy link
Collaborator

I agree, we can change the default parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants