Add nested namespaces

[stephprince]

Fix #14 and #3.

We mentioned previously that we wanted to consider adding sub-namespaces to the main AQNWB namespace (see here). For example, adding something like AQNWB::HDF5::HDF5IO to isolate the backends (see here).

If we want to add nested namespaces, here were a couple options I was thinking of for how to structure these:

1. Add AQNWB::HDF5 and AQNWB::Schema sub namespaces with everything else in the main namespace. Additional backends would get their own subnamespaces. The current version of the PR implements this - adding the nested Schema and HDF5 namespaces and restructuring schema files into a single schema subfolder.
2. Have AQNWB::IO, AQNWB::IO::HDF5, AQNWB::Schema::HDMF, AQNWB::Schema::NWB namespaces. This more closely matches the folder structure but adds deeper nesting which might make it less readable. (A slightly related question, do we want the folder naming to be more like pynwb with nwb-schema/core and nwb-schema/hdmf-common-schema subfolders?)

Not sure if it's worth noting but the nested namespace definitions (namespace A::B::C) were added in C++17 so if we want to be compatible with older compilers we would need the longer, separate namespace definitions (e.g. namespace A { namespace B { namespace C { }}}).

[oruebel]

Thanks for the PR! The approach looks overall good to me. A couple of suggestions below.

Not sure if it's worth noting but the nested namespace definitions (namespace A::B::C) were added in C++17 so if we want to be compatible with older compilers we would need the longer, separate namespace definitions (e.g. namespace A { namespace B { namespace C { }}}).

I would go with the longer definition of namespace A { namespace B . I don't think the short-hand adds that much that it's worth requiring C++ 17 for.

1. Add AQNWB::HDF5 and AQNWB::Schema sub namespaces with everything else in the main namespace.

I agree, going too deep with the nesting is not useful at this point and this strikes a nice balance between separating concerns with namespaces while keeping the code readable by avoiding too many namespaces. 👍

I would suggest the following changes:

• Instead of schema I think calling the namespace nwb would be more intuitive for users and also leave the term schema open in case we need to eventually add functionality for interacting with schema
• I would match the folder structure with the namespaces structure, i.e., have two subfolders: nwb and hdf5 and have all the files for that namespace in those folders. I think it's Ok to have subfolders within those folders if necessary that are not separate sub-namespaces if necessary. To match the folder structure, I think this means that
  • BaseIO would move from hdf5 to the main folder
  • NWBFile would move from the main folder to the nwb namespace

[oruebel]

2. Have AQNWB::IO, AQNWB::IO::HDF5, AQNWB::Schema::HDMF, AQNWB::Schema::NWB namespaces. This more closely matches the folder structure but adds deeper nesting which might make it less readable

If you would like to use more nested namespaces with matching folder structure, but still keep code readable, we could use inline namespace declarations for the nested namespaces. Here a brief discussion about inlining namespaces https://www.geeksforgeeks.org/cpp-inline-namespaces-and-usage-of-the-using-directive-inside-namespaces/?ref=lbp I just wanted to mention it as an additional option, but I think your proposal of having two main namespaces is fine.

[stephprince]

Sounds good! I changed the name to AQNWB:NWB namespace and updated the folder structure accordingly.

I would go with the longer definition of namespace A { namespace B . I don't think the short-hand adds that much that it's worth requiring C++ 17 for.

Wanted to add that I realized there was at least one other place where I've used C++17 features (mainly when using the std::filesystem library). Would we need to support older compilers? I believe OpenEphys uses C++17 for all plugins.

[oruebel]

Wanted to add that I realized there was at least one other place where I've used C++17 features (mainly when using the std::filesystem library). Would we need to support older compilers? I believe OpenEphys uses C++17 for all plugins.

We should look at some of the acquisition system software what they allow. I suspect that C++11 is fine but I wouldn't be surprised if C++17 is not yet supported. In general I think if it's not adding significant value, then avoiding C+17 features seems preferable.

[oruebel]

I added a couple of additional comments, but otherwise this looks good.

[oruebel]

I suspect that C++11 is fine but I wouldn't be surprised if C++17

Digging around the web a bit more, it looks like C++17 should be fine with most compilers less than 3-5years old. So C++17 I think should be Ok. https://en.cppreference.com/w/cpp/compiler_support/17

I think we should avoid C++ 20 features as compiler support seems to be still not as mature and widespread. E.g., the GCC status page says "Because the ISO C++20 standard is very recent, GCC's support is experimental" (see https://gcc.gnu.org/projects/cxx-status.html). The compiler list also still shows a bunch of red for C++ 20 https://en.cppreference.com/w/Template:cpp/compiler_support/20 . Also, to use C++ 20 seem to still required additional build parameters, e.g., -std=c++20, -std=c++2a, or -std=gnu++20.

C++ 23 and 26 are too recent and support is still too spotty so we should not use those features yet https://en.cppreference.com/w/cpp/compiler_support

[oruebel]

Are these using still necessary? Just wondering, because BaseIO and Types are now in the same namespace?

[stephprince]

I was using it more to create a type alias for Status, SizeArray, and SizeType, but I can replace the usage with Types::Status throughout BaseIO, HDF5IO if that's better?

[oruebel]

Thanks for the clarification. Makes sense. This is fine.

[stephprince]

Got it, that's a really helpful page for looking at the compiler support. I can change back the namespace definitions then if we think C++17 should be ok

[oruebel]

Looks good to me

[oruebel]

I can change back the namespace definitions then if we think C++17 should be ok

Whichever style you prefer for the namespace definitions is fine with me.

[stephprince]

Ok, I have a slight preference for the newer style just in case we ever need deeper nesting so I will go with that!