Support for local tables in separated directory from root dir

[ReallyNiceGuy]

Behavior change on command line tool:

New command line switches:

-l -> Path for local tables
-p -> Provider of local table

The user can select a separate path and provider for local tables.

Behavior change on library:
A new parameter was added to Encode and Decode classes:
tables_local_dir: Path to search for local tables. Default is the same value as tables_root_dir. Here the path should already contain the provider of the table.

Suggest directory structure:

pybufrkit/tables
└── 0
    ├── 0_0
    └── 98_0

some_other_path/tables
└── 0
    ├── imd   <-- new local table with a provider name for selection
    │   └── 255_255
    └── meteofrance   <-- another new local table
    │   └── 42_0
    └── internal  <-- another new local table, with clashing table numbers with provider imd
        └── 255_255

[ywangd]

Mostly looking good. I have some minor comments that I'd appreciate if they can be addressed. Thanks!

[ywangd]

Nit: default=None is redundant

[ywangd]

Same here

[ywangd]

Let's use os.path.join(...) since it handles different path separator on different platforms.

[ywangd]

I think this should be

Suggested change

	self.tables_local_dir = tables_local_dir or tables_root_dir
	self.tables_local_dir = tables_local_dir or self.tables_root_dir

Otherwise it could still be None.

[ReallyNiceGuy]

I will rebase and do the requested changes later today.

…

On Sat, Mar 22, 2025, 02:47 Yang Wang ***@***.***> wrote: ***@***.**** commented on this pull request. Mostly looking good. I have some minor comments that I'd appreciate if they can be addressed. Thanks! ------------------------------ In pybufrkit/__init__.py <#41 (comment)>: > @@ -62,6 +62,14 @@ def main(): ap.add_argument('-t', '--tables-root-directory', help='The directory to locate BUFR tables') + ap.add_argument('-l', '--tables-local-directory', + help='The directory to locate local BUFR tables', + default=None) Nit: default=None is redundant ------------------------------ In pybufrkit/__init__.py <#41 (comment)>: > @@ -62,6 +62,14 @@ def main(): ap.add_argument('-t', '--tables-root-directory', help='The directory to locate BUFR tables') + ap.add_argument('-l', '--tables-local-directory', + help='The directory to locate local BUFR tables', + default=None) + + ap.add_argument('-p', '--tables-local-provider', + help='The provider for local BUFR tables', + default=None) Same here ------------------------------ In pybufrkit/__init__.py <#41 (comment)>: > @@ -261,6 +269,10 @@ def main(): ns = ap.parse_args() + ns.tables_local_directory = ns.tables_local_directory or ns.tables_root_directory + if ns.tables_local_provider: + ns.tables_local_directory = ns.tables_local_directory + "/" + ns.tables_local_provider Let's use os.path.join(...) since it handles different path separator on different platforms. ------------------------------ In pybufrkit/coder.py <#41 (comment)>: > self.section_configurer = SectionConfigurer(definitions_dir=definitions_dir) self.tables_root_dir = tables_root_dir or DEFAULT_TABLES_DIR + self.tables_local_dir = tables_local_dir or tables_root_dir I think this should be ⬇️ Suggested change - self.tables_local_dir = tables_local_dir or tables_root_dir + self.tables_local_dir = tables_local_dir or self.tables_root_dir Otherwise it could still be None. — Reply to this email directly, view it on GitHub <#41 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABR6CQTBGGS56JUA5SUL5GT2VS6JZAVCNFSM6AAAAABZFMVF5KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMBXGYZDIMBXGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ReallyNiceGuy]

I made the changes and force-pushed it to my branch.
Let me know if you want more changes.

[ywangd]

Thanks. Looks good to me. I'll be merging this.