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

make the band chart/map metadata format be a dictionary? #310

Open
classabbyamp opened this issue Nov 21, 2020 · 3 comments
Open

make the band chart/map metadata format be a dictionary? #310

classabbyamp opened this issue Nov 21, 2020 · 3 comments
Labels
enhancement New feature or request investigate This issue needs investigation refactor This will require refactoring code

Comments

@classabbyamp
Copy link
Member

a dictionary would be more informative at first glance, would probably make it harder to make an error when adding more, and make the qrm-side code simpler.

Example

{
  "cn": {
    "path": "/resources/bandcharts/cn.png",
    "short_name": "China",
    "long_name": "Amateur radio bands in China",
    "description": "",
    "source": "Created by KN8U and NY7H",
    "emoji": "🇨🇳"
  }
}
@0x5c
Copy link
Member

0x5c commented Nov 22, 2020

This will require me looking at the relevant code and checking what was the reason for using lists.
I suspect it was to have a system where omitting a field would cause errors.
But I am not exactly sure why that over something else, or is there was more to it.

@0x5c 0x5c transferred this issue from miaowware/qrm-resources Nov 22, 2020
@0x5c
Copy link
Member

0x5c commented Nov 22, 2020

This would come after I'm done with the actual move (#246) in the existing code

@0x5c 0x5c added old/question Further information is requested refactor This will require refactoring code enhancement New feature or request labels Nov 22, 2020
@0x5c 0x5c added investigate This issue needs investigation and removed old/question Further information is requested labels Apr 2, 2021
@0x5c
Copy link
Member

0x5c commented Apr 2, 2021

Investigation notes:
IIRC the issue with this idea was dataclasses being trash at dealing with supplementary kwargs, which would cause issue at data unpacking if the resource file gains a new key.
I think that this was the only issue, and it would be fixed by just using pydantic instead: it's already in the requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request investigate This issue needs investigation refactor This will require refactoring code
Projects
None yet
Development

No branches or pull requests

2 participants