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

Export all constants? #1

Closed
giordano opened this issue Sep 9, 2018 · 27 comments
Closed

Export all constants? #1

giordano opened this issue Sep 9, 2018 · 27 comments
Labels
help wanted Extra attention is needed

Comments

@giordano
Copy link
Member

giordano commented Sep 9, 2018

Currently the submodule CODATA2014 exports all constants. This definitely clutters Main with some short names. On the other end, without automatic exports, users would have to spell in full PhysicalConstants.CODATA2014.c, unless they do something like

julia> using PhysicalConstants, PhysicalConstants.CODATA2014

julia> const cst = PhysicalConstants.CODATA2014
PhysicalConstants.CODATA2014

julia> cst.c
Speed of light in vacuum (c)
Value                         = 2.99792458e8 m s^-1
Standard uncertainty          = (exact)
Relative standard uncertainty = (exact)
Reference                     = CODATA 2014    

which may not be very convenient.

A good point of not exporting the constants is that it would be easy to load two different datasets providing different values for the same constants (e.g., other CODATA datasets) and use values from one module or another without messing things up.

I'm not sure what's the best thing to do here.

@giordano giordano added the help wanted Extra attention is needed label Sep 9, 2018
@giordano giordano changed the title Design discussion Export all constants? Sep 9, 2018
@carstenbauer
Copy link
Member

Hi, first of all very useful package!

What's your reasoning behind supporting multiple datasets in the first place? These are constants after all and I feel we should always use the most accurate results available and not confuse the user by having to choose a dataset. At least as a default. I, for example, had never heard of CODATA2014.

Regarding this issue, we could then just export a global const constants or similar, which holds all the constants. (In PeriodicTable.jl we do the same with elements.

@giordano
Copy link
Member Author

What's your reasoning behind supporting multiple datasets in the first place

For example backward compatibility: one may need to continue working with a specific dataset, without holding back the whole package. And can then switch to a different dataset just by using a different module.

In addition, CODATA is the only dataset I know, but there can be different projects to measure fundamental constants

@giordano
Copy link
Member Author

Also astropy uses different modules for different datasets(e.g., different editions of the same project): http://docs.astropy.org/en/stable/constants/

@Datseris
Copy link

Datseris commented Sep 10, 2018

-1 for exporting many small names. I don't think anything should be exported by this package. Instead it is best to do (as a user)

using Constants.C0DATA2014: c, \hbar, ...

Such small names are typically easily already in use. So even if someone wants the speed of light, but already has bound c to something important relevant to their algorithm, they can still do

speed_of_light = Constants.C0DATA014.c

edit: Scratch that: I can already see that no name is exported by this package besides the C0DATA2014. So you can still use Constants without polluting your name space. As long as you don't do using Constants.CODATA.

@giordano
Copy link
Member Author

I'm starting to like the suggestion by @crstnbr to export a single object that can be used to reference all constants. I've came up with a crazy idea on how to do this, I'll see if this works.

@Datseris actually, currently all constants are exported by the CODATA submodule, this is done under the hood by the @constant macro 😉 However, I'm the first one not very happy about this solution

@giordano
Copy link
Member Author

Would make it sense to split the datasets into separate packages and keep here only the basic infrastructure to define constants? Fox example, I'm thinking about defining astronomical constants, but they probably wouldn't fit here. In other Physics subfields people may want other constants, that are of general interest.

In this way, the name of the module would be much shorter (no PhysicalConstants.*) and easier to do something like CODATA2014.c (or CODATA.2014.c, with different submodules for each edition of CODATA constants).

@Datseris
Copy link

Datseris commented Sep 11, 2018

This becomes too convoluted. I don't see any reason for this complexity. Less repos = easier to handle.

It's clear that this package will have 1-2 dependencies (Unitful) and only text files as a source. 100 more lines for more constants do not seem like a deal to me. I prefer 1 package with many submodules.

It is not a big deal to do:

using PhysicalConstants.Astro: a, b, c

And also make all submodules export all their constants. This way you can still either only use the ones you want, with the syntax above, or you can use all of them by doing

using PhysicalConstants.Astro

It is kind of clear that different submodules would have different constants sets. And even if it isn't, as long as it is documented, then it is.

@giordano
Copy link
Member Author

Ok ok, makes sense, thank you!

I've just realized that the name of the submodule is automatically exported when you load it, I thought it wasn't! So right now you can already do:

julia> using PhysicalConstants.CODATA2014

julia> CODATA2014.c
Speed of light in vacuum (c)
Value                         = 2.99792458e8 m s^-1
Standard uncertainty          = (exact)
Relative standard uncertainty = (exact)
Reference                     = CODATA 2014

Does it sound like a good solution? In this case I'd just have to not export the constants when defining them inside the macro.

@Datseris
Copy link

What my personal vote for the API is:

  1. PhysicalConstants exports nothing. It has a bunch of submodules, each one a well-defined (whatever well-defined means here) set of constants.
  2. Each submodule of PhysicalConstants exports all of its constants.

This is nice because it allows the following:

  1. using PhysicalConstants.Astro: a, b, c brings into scope only the constants I want as a user.
  2. using PhysicalConstants.Astro brings into scope all constants from the set Astro.
  3. using PhysicalConstants by itself is meaningless. And in my eyes it should be, because its only meaningful function could be to export "all" physical constants that exist. See below for more on this!

After this is done and published, we can make a poll of which module to be the default, i.e. inside the source we can write

using Reexport
@reexport DefaultSet

which would allow using PhysicalConstants to equate using PhysicalConstants.DefaultSet.

@carstenbauer
Copy link
Member

carstenbauer commented Sep 11, 2018

I'm not sure whether I'm a fan of the

using PhysicalConstants.Astro: a, b, c

approach. I guess it's because I don't really like the fact that I'd have to tell the package what constants I would like to get on loading the package. I might not know in advance which constants I'll need. The only alternative would be to load all of them and pollute the namespace. Also, how would I get an overview of all available constants in a (sub)module from within the REPL?

Regarding the categorical submodules (like Astro), this should only be used for (very) specific constants. I'd hate it to have to figure out which constant lives in which submodule.

@Datseris
Copy link

Datseris commented Sep 11, 2018

Also, how would I get an overview of all available constants in a (sub)module from within the REPL?

names(PhysicalConstants.DataSet).

Regarding the categorical submodules (like Astro), this should only be used for (very) specific constants. I'd hate it to have to figure out which constant lives in which submodule.

Documentation by Documenter has search functionality! In any case it's only these two options that are possible:

  1. Pollute the namespace with a crazy bunch of names
  2. Have everything separated.

EDIT: The comment

I might not know in advance which constants I'll need.

has little strength to it: for the interactive case, if you don't know in advance which constants you need it is by far the safest approach to import them one by one than to immediatelly load 100 names which you won't even know how they are called, since you don't know in advance.

For the scripting case, it has no meaning whether you know them in advance or not. You still put them in the first line of your script :D

@carstenbauer
Copy link
Member

carstenbauer commented Sep 11, 2018

names(PhysicalConstants.DataSet).

This would only work after using PhysicalConstants, right? So this better shouldn't reexport any constants then I guess.

To submodules: One just shouldn't over-categorize. Apart from that I agree that separating is the better choice.

@carstenbauer
Copy link
Member

carstenbauer commented Sep 11, 2018

has little strength to it: for the interactive case, if you don't know in advance which constants you need it is by far the safest approach to import them one by one than to immediatelly load 100 names which you won't even know how they are called, since you don't know in advance.

Not knowing which ones I need doesn't mean I don't know how they are called when I need them. But I also wasn't suggesting to load all of them to global namespace in this case. An alternative could be to export a container Constants which in a sense defines a namespace for constants. And in case I really need to use a couple of constants a lot I could always say const c = Constants.c or so. But maybe this is more annoying when I know which constants I want.

@Datseris
Copy link

Datseris commented Sep 11, 2018

This would only work after using PhysicalConstants, right? So this better shouldn't reexport any constants then I guess.

Yes yes this is also exactly why I suggested PhysicalConstants to export nothing by default.

aaaafter this package has seen some use, then we can consider a specific dataset to be the default, or not at all.


One just shouldn't over-categorize

I totally agree, although @giordano made a very strong point of backwards compatibility with the example of the C0DATA. Such cases are important, and verbosity should be preferred.


An alternative could be to export a container Constants which in a sense defines a namespace for constants. And in case I really need to use a couple of constants a lot I could always say const c = Constants.c or so.

I don't see how this is different from having a module... In fact it is identical. I can't tell any difference with what you suggested and what I suggested :D Unless you mean to have one extra "global" like module, like AllConstants that exports all constants from all modules? This can lead to many problems, since some constants may share the same name. This is also impossible in the case we want support for backwards compatibility, or in general, support of the past.

@giordano
Copy link
Member Author

I think we all agree that the base module shouldn't export anything. It's the current situation and it's documented in the README.md that users most probably won't need to do using PhysicalConstants.

@crstnbr The container can be the submodule itself, like in my example above: when you load a submodule its name is exported. This would also have the advantage of featuring auto-completion in the REPL:

julia> using PhysicalConstants.CODATA2014

julia> CODATA2014.m_
m_e m_n  m_p  m_u
julia> CODATA2014.m_e
Electron mass (m_e)
Value                         = 9.10938356e-31 kg
Standard uncertainty          = 1.1e-38 kg
Relative standard uncertainty = 1.2e-8
Reference                     = CODATA 2014

It's clear for users what constants they're referring to (electron mass from CODATA2014, rather another m_e from a different datasets, maybe something completely different from electron mass) and we don't need to define a container whose name may conflict with the container name of another module.

@carstenbauer
Copy link
Member

carstenbauer commented Sep 11, 2018

Your use case is different from what @Datseris suggested in that the submodule CODATA2014 doesn't export anything as well. I think I like that better: either specify what you want explicitly or you get a container (the submodule itself).

EDIT2:

Also, I really see no use case where someone would need all constants of a submodule.

EDIT:

We could even control getproperty and propertynames to only show constants in autocompletion and hide potential internal variables/functions.

@carstenbauer
Copy link
Member

carstenbauer commented Sep 11, 2018

It's the current situation and it's documented in the README.md that users most probably won't need to do using PhysicalConstants.

I'm basically thinking of the (dumb) user scenario where a user just wants to get some of the most important physical constants (speed of light, hbar and so on) as simply as possible. He wouldn't know about any dataset or category submodules that we came up with. Basically, what he wants to do is just using PhysicalConstants and have his constants ready in some form. But I guess it's just a matter of documentation.

@giordano giordano mentioned this issue Sep 14, 2018
@adamslc
Copy link

adamslc commented Oct 16, 2018

I don't think that it makes sense to not export anything when a user types using PhysicalConstants. In fact, my preferred behavior would be to import all of the constants. This would undoubtedly clutter the namespace, but (and I think more importantly) it would also allow users to quickly enter expressions using the constants.

If users, or especially package authors are worried about cluttering their namespace, then they should include PhysicalConstants.

@Paalon
Copy link

Paalon commented Oct 23, 2018

I am user of physical constants (not this module) and there are no standard module for them due to many packages lack in function, so I implemented module for myself. In my module,
https://github.com/Paalon/CODATA2014.jl
I only exported constants by their name (like SpeedOfLightInVacuum) and exported Constants to have all information of CODATA2014, and some frequently used constants has symbolic names definitions but not exported.

# freqeuntly used constants defined in CODATA
export
    AtomicMassConstant,
    AvogadroConstant,
    BoltzmannConstant,
    ConductanceQuantum,
    ElectricConstant,
    ElectronMass,
    ElectronVolt,
    ElementaryCharge,
    FaradayConstant,
    FineStructureConstant,
    InverseFineStructureConstant,
    MagneticConstant,
    MagneticFluxQuantum,
    MolarGasConstant,
    NewtonianConstantOfGravitation,
    PlanckConstant,
    PlanckConstantOver2pi,
    ProtonMass,
    ProtonElectronMassRatio,
    RydbergConstan,
    SpeedOfLightInVacuum,
    StefanBoltzmannConstant

I think we should export their (natural language) name and symbolic name should not be exported. Anyway I want this module to have all CODATA2014 values (https://github.com/Paalon/CODATA2014.jl/blob/master/src/allascii.csv) because this doesn't have all of them.

@giordano
Copy link
Member Author

I've been quite busy lately and this will last other ~15 days, after that I hope to be able to resume this discussion. I just want to quickly reply to this comment:

I don't think that it makes sense to not export anything when a user types using PhysicalConstants.

The rationale about this decision is that the package contains different submodules with different datasets, which can also define different values for the same physical constant (like different versions of CODATA). Users will do using PhysicalConstants.DATASETNAMEHERE. With this approach, I don't think it'll be useful to import by default anything when using PhysicalConstants.

The discussion here is whether to export all constants in the submodules or not. I think I kind of like @Paalon's suggestion to export long names.

@giordano
Copy link
Member Author

My ~15 days lasted quite a long time 😅

Now that #2 is fixed, Constants can be readily used in mathematical operations without having to manually convert them to a Quantity, and we can go back to this issue.

To summarise, I think we all agree that using PhysicalConstants shouldn't export anything and users would instead load a specific dataset.

Now, my question is: what do we export from the dataset submodule? How do we name the constants: long descriptive names or (possibly very) short abbreviations? I think these questions are kind of linked, because polluting the main workspace with several short names is not going to be fun, on the other hand many users would prefer referring to the speed of light as c rather than SpeedOfLightInVacuum.

@BeastyBlacksmith
Copy link

IMO long descriptive names should be used for the type definition and adding short handle like const c = SeedOfLightInVacuum does not hurt.
Besides the few constants I use regularly like g, h, and k_B I find it hard to remember which letter corresponded to what constant. ( what was \Phi_0 again, what was the difference of R and R_\infty, ... )

@giordano
Copy link
Member Author

IMO long descriptive names should be used for the type definition and adding short handle like const c = SeedOfLightInVacuum does not hurt.

It's not clear to me whether you're suggesting that the package or the users should define the shorthands.

Besides the few constants I use regularly like g, h, and k_B I find it hard to remember which letter corresponded to what constant. ( what was \Phi_0 again, what was the difference of R and R_\infty, ... )

I agree on this, it's quite unlikely that someone is going to frequently use all the available constants and will probably care only about a few of them. But defining a minimum set of common constants is totally arbitrary and everyone would complain "why X is shortened but not Y? I use Y all the time and I don't want X shortened".

@BeastyBlacksmith
Copy link

I meant having the shorthands in the package, but not export them. This way we don't litter other peoples namespaces and the user can still write using PhysicalConstants.DATABASE: c,h,g.

@giordano
Copy link
Member Author

So your idea would be to provide non-exported shorthand for some constants? I think I like it.

@BeastyBlacksmith
Copy link

BeastyBlacksmith commented Mar 14, 2019

Could be even for all, if there is a common used one.
Alternatively add the obvious ones and add additional ones by request.

@giordano
Copy link
Member Author

I've implemented in #7 the ideas suggested here. The README.md summarises the new strategy:

  • the dataset modules will export all the constants with their long descriptive names
  • all constants have short aliases that are not exported, but can be brought to the Main namespaces with an import

Note also that without explicitly importing anything, one could do also something like this:

julia> using PhysicalConstants

julia> const cnst = PhysicalConstants.CODATA2014
PhysicalConstants.CODATA2014

julia> cnst.c_0
Speed of light in vacuum (c_0)
Value                         = 2.99792458e8 m s^-1
Standard uncertainty          = (exact)
Relative standard uncertainty = (exact)
Reference                     = CODATA 2014

julia> cnst.G
Newtonian constant of gravitation (G)
Value                         = 6.67408e-11 m^3 kg^-1 s^-2
Standard uncertainty          = 3.1e-15 m^3 kg^-1 s^-2
Relative standard uncertainty = 4.6e-5
Reference                     = CODATA 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants