Picongpu: Should we rename namespace picongpu::openPMD ?

Created on 15 Oct 2020  Â·  14Comments  Â·  Source: ComputationalRadiationPhysics/picongpu

It causes to use openPDD API as ::openPMD:: and for those who are unaware this may cause trouble e.g. when copying from openPMD API examples

plugin question

Most helpful comment

If I'm interpreting the discussion correctly, @sbastrakov and @psychocoderHPC 's objections were to having namespaces picongpu::openPMD and io = openPMD.
I would go for picongpu::io and leaving openPMD untouched, but if picongpu::openPMDHelper is preferred to keep things in the same style, I'm fine with that too ;)

All 14 comments

To clarify it is also (in my opinion) difficult to quickly read the code that uses both ::openPMD:: and openPMD, often in the same expression.

So I would prefer if picongpu::openPMD would become smth like picongpu::openPMDHelper.

Potentially.
Also I recommend doing this after #include:

namespace io = openPMD;

which makes it unique, too ;D

I think while there are other IO libraries in use that would be a weird alias to use

I mean, weird ourside of the openPMD "land", as one reading PIConGPU code would need to know io means oprnPMD. And having at the same time picongpu::openPMD does not become less confusing then imo

I for changing picongpu::openPMD to picongpu::openPMDHelper.
namespace io = openPMD; is also possible but I think we do not have so many openPMD interactions in the code so being a little bit more verbose and writeing openPMD::* is better.

Axel's suggestion would mean to rename the original openPMD namespace (i.e. the one by the openPMD API) as io? I agree that this might be confusing, I would leave that namespace untouched. What about renaming picongpu::openPMD as picongpu::io instead? That would make sense imo since the openPMD plugin is in fact PIConGPU's IO plugin.
I don't really like openPMDHelper, things like *helper or *detail always sound too much like it's some little namespace that you're not supposed to be touching, while in reality it's the complete plugin…

Axel's suggestion would mean to rename the original openPMD namespace (i.e. the one by the openPMD API) as io?

Not sure if we mean the same thing, probably. This is just an example of how import numpy as np and import matplotlib.pyplot as plt looks in C++ inside user code. I use this consistently in examples in the docs because it makes code nice and concise. There is no need to follow it, it's just a short-hand and my ;D indicated the half-joke.

I agree with René's suggestion. I just mentioned this because namespace aliases are pretty and too seldomly used :)

No, this is just an example of how import numpy as np looks in C++ inside _user_ code. I use this consistently in all examples in the docs because it makes code nice and concise. There is no need to follow it, it's just a short-hand.

Yeah, but this would be the effect of it, right? Renaming openPMD's openPMD namespace as io within PIConGPU, so PIConGPU can use its own picongpu::openPMD namespace without having things clash?
No issue with namespace io = openPMD in general, but introducing our own picongpu::openPMD namespace afterwards is asking for trouble :D If we want to rename things, picongpu::openPMD is the better candidate imo

Yep, I think renaming it to "...detail" is not super pretty but ok and already used throughout the code base.

If I'm interpreting the discussion correctly, @sbastrakov and @psychocoderHPC 's objections were to having namespaces picongpu::openPMD and io = openPMD.
I would go for picongpu::io and leaving openPMD untouched, but if picongpu::openPMDHelper is preferred to keep things in the same style, I'm fine with that too ;)

Sorry, when creating this issue and discussing it before I was confusing two things. So there is picongpu::openPMD namespace for the openPMD plugin. And it was also (by coincidence?) used for the ParticlePatches class. And I meant to rename that one to openPMDHelper, and maybe add some more stuff there, not to rename the plugin namespace, which is how everyone understood that probably.

But now I've figured the ParticlePatches class is not used and made #3419 to remove it.

And for the openPMD plugin, maybe it could become picongpu::plugins::openPMD (currently some plugins have that plugins in between but most do not), then I think it would be much harder to confuse with openPMD API.

If we rename picongpu::openPMD as picongpu::plugins::openPMD, this won't really solve the issue with having to use ::openPMD for the original openPMD namespace though?

Yes, it won't really for usage of openPMD API inside the plugin, which is most of the usage right now.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bussmann picture bussmann  Â·  4Comments

berceanu picture berceanu  Â·  4Comments

ax3l picture ax3l  Â·  4Comments

HighIander picture HighIander  Â·  4Comments

sbastrakov picture sbastrakov  Â·  3Comments