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

global default musings #339

Open
gmabey opened this issue Feb 6, 2025 · 2 comments
Open

global default musings #339

gmabey opened this issue Feb 6, 2025 · 2 comments

Comments

@gmabey
Copy link
Contributor

gmabey commented Feb 6, 2025

I've been looking at the global properties, and have some questions -- apologies if these have been already beat-to-death in some other forum.

  1. I don't think that core:data_type should both be required and have a default value. Could say the same about core:sample_start I guess.
  2. Looks like the leading r in core:data_type is now required, whereas it used to be implicit. Was that intentional, or a byproduct of trying to write a regexp that describes the construction of that field?
  3. I suppose that core:trailing_bytes doesn't have a default=0 because it's only to be used with NCDs, but why not?
  4. I thought that there was an analog to core:trailing_bytes to indicate "skip this number of bytes in the NCD file before reaching the first sample value". I'm thinking of the use case of generating a .sigmf-meta file describing an NCD that is a MIDAS-Blue file, which I'm sure has a header (besides a footer).
  5. Did core:sample_rate used to be required? I think I would vote for it to be required.
  6. I did once work on a project that had a very slow (geologic) sample rate. Suggest changing core:sample_rate -> "minimum" to 1e-12.
@Teque5
Copy link
Collaborator

Teque5 commented Feb 6, 2025

  1. The defaults were created when we thought we were going to read them to insert default values into constructors. I don't think we actually need any of them anymore.
  2. I think having c or r first has always been the case.
  3. goto 1
  4. core:header_bytes
  5. Yes it should be required.
  6. If you look here it's actually possible to specify x > 0, so we should do that

gmabey added a commit to gmabey/SigMF that referenced this issue Feb 6, 2025
gmabey added a commit to gmabey/SigMF that referenced this issue Feb 6, 2025
@gmabey
Copy link
Contributor Author

gmabey commented Feb 6, 2025

  1. The defaults were created when we thought we were going to read them to insert default values into constructors. I don't think we actually need any of them anymore.

Do we still want a missing core:num_channels to imply 1? I don't think they hurt anything, and by specifying a default, we allow .sigmf-meta files to be more concise/cleaner when those fields don't apply. However, taking that to an extreme it gets silly, like a default for core:metadata_only of false ...

I have attempted a judicious removal of some of the default lines. Feedback welcome.

  1. I think having c or r first has always been the case.

I bet it's from the python code itself that I had that impression.

  1. goto 1
  2. core:header_bytes

(facepalm)

  1. Yes it should be required.
  2. If you look here it's actually possible to specify x > 0, so we should do that

Nice.

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

No branches or pull requests

2 participants