-
Notifications
You must be signed in to change notification settings - Fork 30
Implemented local tables selection flag and helper function #40
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
Conversation
This changes allow us to add local tables directly into the tables directory.
|
IIUC, the intention is to use local tables along side WMO tables. This is already supported since the code loads local tables as defined in section 1 and the There it should work by placing the new local tables alongside the existing ones, e.g.: Let me know if the above works for you. |
|
It almost works. I ran in the case where 2 different entities used the same
local tables with different definitios. With the current solution, I would
have to select which one of them to add to the tables directory. My change
allows for tables redefinitions without clashes.
I am open to suggestions on how to solve this clash issue without changing
the table structure.
Thank you.
…On Sat, Mar 15, 2025, 00:10 Yang Wang ***@***.***> wrote:
IIUC, the intention is to use local tables along side WMO tables. This is
already supported since the code loads
<https://github.com/ywangd/pybufrkit/blob/7d1ff093bdac80d7d653cc724ebc10c05bc7f10a/pybufrkit/tables.py#L187-L191>
local tables as defined in section 1
<https://github.com/ywangd/pybufrkit/blob/7d1ff093bdac80d7d653cc724ebc10c05bc7f10a/pybufrkit/definitions/section1-4.json#L19-L28>
and the tables directory is structured to accomodate extra local tables.
The current tables directory is as the follows:
tables/
└── 0 <-- master table number as defined in section 1
├── 0_0 <-- originating centre and sub-centre as defined in section 1, 0_0 for WMO
└── 98_0 <--- originating centre and sub-centre for ECMWF
There it should work by placing the new local tables alongside the
existing ones, e.g.:
tables/
└── 0
├── 0_0
├── 98_0
├── 85_0
└── 255_255
Let me know if the above works for you.
—
Reply to this email directly, view it on GitHub
<#40 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABR6CQT4FVE2JXVGBDWIHA32UNOVLAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRVHE4DAOJUG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: ywangd]*ywangd* left a comment (ywangd/pybufrkit#40)
<#40 (comment)>
IIUC, the intention is to use local tables along side WMO tables. This is
already supported since the code loads
<https://github.com/ywangd/pybufrkit/blob/7d1ff093bdac80d7d653cc724ebc10c05bc7f10a/pybufrkit/tables.py#L187-L191>
local tables as defined in section 1
<https://github.com/ywangd/pybufrkit/blob/7d1ff093bdac80d7d653cc724ebc10c05bc7f10a/pybufrkit/definitions/section1-4.json#L19-L28>
and the tables directory is structured to accomodate extra local tables.
The current tables directory is as the follows:
tables/
└── 0 <-- master table number as defined in section 1
├── 0_0 <-- originating centre and sub-centre as defined in section 1, 0_0 for WMO
└── 98_0 <--- originating centre and sub-centre for ECMWF
There it should work by placing the new local tables alongside the
existing ones, e.g.:
tables/
└── 0
├── 0_0
├── 98_0
├── 85_0
└── 255_255
Let me know if the above works for you.
—
Reply to this email directly, view it on GitHub
<#40 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABR6CQT4FVE2JXVGBDWIHA32UNOVLAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRVHE4DAOJUG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Thanks for explaining. Do you mean two entities use exactly the same codes for originating centre and originate sub-centre? Is that valid in WMO specs? I was assuming at least the code for originating centre should be uniquely assigned to different entities? Maybe that is not true? If that is the case, is there a reason why this cannot be solved with the existing where both The difference betwen the above and what you are proposing is the need to specify the full tables path instead of a shorter name. I am not sure how significant this difference is since there are multiple ways you can make the full path shorter, e.g. define an environment variable for it, e.g. |
|
Yes, you understood it correctly. It is not valid, but this will not prevent the end user from making this mistake, unfortunately. Your example was exactly my first solution. If you are fine with changing the structure to be: I can definitely live with that. The helper function and switch are just "easy of use" anyways. I saw that you just committed some changes to download the WMO tables. This will have to be addressed also with such a change, and my commit doesn't do it. The reason for the links is because of another crazy thing I saw in the wild: overriding codes from WMO. My approach to that would be to just create a modified WMO table with the override and put it directly inside the local table directory for the entity that dares to do that. Again, thank you for taking the time to work this out with me. |
|
I may have been unclear in the previous comment. I'd prefer to not change the default directory layout. My view is that non-standard tables are better managed outside of the program itself. The purpose of the builtin tables is mostly there for a good get-started experience with standard WMO tables. Once you have serious need for local tables, it's better to manage them separately with more nuances, e.g. with the above suggested structure. The existing |
|
I understand what you mean now.
I will remove the pull request.
Thank you anyway.
…On Sat, Mar 15, 2025, 12:47 Yang Wang ***@***.***> wrote:
I may have been unclear in the previous comment. I'd prefer to *not*
change the default directory layout. My view is that non-standard tables
are better managed outside of the program itself. The purpose of the
builtin tables is mostly there for a good get-started experience with
standard WMO tables. Once you have serious need for local tables, it's
better to manage them separately with more nuances, e.g. with the above
suggested structure. The existing -t option already supports loading
tables from arbitrary locations. Hence I don't see the need for changes
proposed in the PR. I hope that makes sense. Thank you!
—
Reply to this email directly, view it on GitHub
<#40 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABR6CQRYZOG46LD36CZLQQ32UQHOXAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRWGQ3TAOBVGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: ywangd]*ywangd* left a comment (ywangd/pybufrkit#40)
<#40 (comment)>
I may have been unclear in the previous comment. I'd prefer to *not*
change the default directory layout. My view is that non-standard tables
are better managed outside of the program itself. The purpose of the
builtin tables is mostly there for a good get-started experience with
standard WMO tables. Once you have serious need for local tables, it's
better to manage them separately with more nuances, e.g. with the above
suggested structure. The existing -t option already supports loading
tables from arbitrary locations. Hence I don't see the need for changes
proposed in the PR. I hope that makes sense. Thank you!
—
Reply to this email directly, view it on GitHub
<#40 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABR6CQRYZOG46LD36CZLQQ32UQHOXAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRWGQ3TAOBVGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I just thought, can I get an extra path for local tables search? This way I
can decouple the main tables from the local tables and I can make my own
packages just for the tables, according to my needs.
if I need to override the wmo tables, I just adjust the main path as it is
done already.
If you would accept this option, I can work in a new pull request.
…On Sat, Mar 15, 2025, 12:51 Marco Costa ***@***.***> wrote:
I understand what you mean now.
I will remove the pull request.
Thank you anyway.
On Sat, Mar 15, 2025, 12:47 Yang Wang ***@***.***> wrote:
> I may have been unclear in the previous comment. I'd prefer to *not*
> change the default directory layout. My view is that non-standard tables
> are better managed outside of the program itself. The purpose of the
> builtin tables is mostly there for a good get-started experience with
> standard WMO tables. Once you have serious need for local tables, it's
> better to manage them separately with more nuances, e.g. with the above
> suggested structure. The existing -t option already supports loading
> tables from arbitrary locations. Hence I don't see the need for changes
> proposed in the PR. I hope that makes sense. Thank you!
>
> —
> Reply to this email directly, view it on GitHub
> <#40 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABR6CQRYZOG46LD36CZLQQ32UQHOXAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRWGQ3TAOBVGU>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
> [image: ywangd]*ywangd* left a comment (ywangd/pybufrkit#40)
> <#40 (comment)>
>
> I may have been unclear in the previous comment. I'd prefer to *not*
> change the default directory layout. My view is that non-standard tables
> are better managed outside of the program itself. The purpose of the
> builtin tables is mostly there for a good get-started experience with
> standard WMO tables. Once you have serious need for local tables, it's
> better to manage them separately with more nuances, e.g. with the above
> suggested structure. The existing -t option already supports loading
> tables from arbitrary locations. Hence I don't see the need for changes
> proposed in the PR. I hope that makes sense. Thank you!
>
> —
> Reply to this email directly, view it on GitHub
> <#40 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABR6CQRYZOG46LD36CZLQQ32UQHOXAVCNFSM6AAAAABZAQWCO6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRWGQ3TAOBVGU>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
IIUC, you are proposing a directory structure as the follows The local tables can then be selected with a new option, say |
|
That would work fine, as long as I can have this structure also separated from the main table: By default, the local path could be the main path as in your example, but the user can select an external path. This would allow me to create packages for each entity, without having to interfere with the |
|
Please check this branch, if you agree with it, I can create a pull request. The changes are:
If you don't like it for any reason, let me know and I will do the changes you feel necessary. |
|
I actually quite like the idea of a separate directory (
Overall the idea is sound to me. Please feel free to open a PR and I will give it a proper review. Thanks! |
|
Actually, 1. is for when we have the local tables directly under the root directory, without an |
This change allows the creation of local tables named by entity in the tables directory.
The table can be selected with the switch
-lin thepybufrkittool or by usingget_local_table_nameto request the local table directory to pass toEncoderorDecoderclasses.