DOC: drastic rewrite of constants documentation #11682

jakobjakobson13 · 2020-03-18T20:53:08Z

If this rewrite is too drastic, let me know.

Reference issue

Fixes #11173

What does this implement/fix?

It is an overhaul of the constants documentation.

Related pull requests

#11343 and #11345 are also concerning the constants module however with a different focus.

If this rewrite is too drastic, let me know.

ilayn · 2020-03-19T09:33:06Z

@jakobjakobson13 Thanks a lot for doing this. Is it possible that you work on this file offline such that it doesn't choke the CI/CD pipeline? Because you are placing a commit each time you make a change it is firing up a new trigger for all the tools as you can see below.

jakobjakobson13 · 2020-03-19T10:29:49Z

@jakobjakobson13 Thanks a lot for doing this. Is it possible that you work on this file offline such that it doesn't choke the CI/CD pipeline? Because you are placing a commit each time you make a change it is firing up a new trigger for all the tools as you can see below.

I did not manage to compile the documentation at my laptop but I'll try it for the next commits. For the moment I would wait anyway for any comments of you guys before I push/make any further commits.

rgommers · 2020-03-19T11:14:50Z

scipy/constants/__init__.py

-
-Force
-----
+or via the dictionaries stored in :mod:`scipy.constants.physical_constants`


This is the cause of the doc build warnings, there is no module physical_constants. Shouldn't this just be scipy.constants?

Ah no, you are wanting to refer to physical_constants, which is simply a dictionary not a module (so both the :mod: is wrong and the sentence may need a tweak:

In [3]: type(constants.physical_constants) Out[3]: dict

rgommers · 2020-03-19T11:18:16Z

You can see the rendered version in the build_docs_artifact CI link. There's still some issues with links.

I like the restructure, first a sensible explanation and the huge list at the end makes sense.

rgommers · 2020-03-19T11:19:47Z

scipy/constants/__init__.py

 ==================
+In 2019 seven système international d’unités (SI) base units were (re)defined to
+serve as a basis for all physical quantities by the bureau
+international des poids et mesures (BIPM) [SIunits]_. They can be summarized as followed


the [ ] brackets need removing.

Well, here I wanted to provide the link with background information. Should I delete it anyway or change its label into "Introduction" or something similar?

No the link is fine, it's just the brackets that are now part of the link and look weird, also SIunits isn't the most informative name. How about simply:

(see [1])

rgommers · 2020-03-19T11:20:33Z

scipy/constants/__init__.py

-``u``              atomic mass constant (in kg)
-``atomic_mass``    atomic mass constant (in kg)
-=================  ============================================================
+- The ampere :math:`\mathrm{A}` as the SI unit of electric current is derived


This would read better as ampere (A) rather than ampere A. And same for next lines.

jakobjakobson13 · 2020-03-19T13:45:08Z

Okay, so CircleCI fails due to release/0.18.0-notes.rst:215: WARNING: py:obj reference target not found: scipy.constants.convert_temperature and TravisCI fails due to ERROR: objects in scipy.constants.__all__ but not in refguide::. The other failures are, as far as I understand, however unrelated to my pull request.

So, how should this be handled?

rgommers · 2020-03-19T16:08:38Z

The convert_temperature one is straightforward, just putting double backticks around that in the 0.18.0 release notes.

For the refguide check, one possible fix is to add all those objects to the skiplist at https://github.com/scipy/scipy/blob/master/tools/refguide_check.py#L127 (in an efficient way, using a list comprehension and a string join, rather than repeating r'scipy\.constants\. over and over).

The argument for the skiplist fix would be that constants don't need separate pages - they don't have docstrings. However, that runs the risk of the list being/becoming incomplete. So probably the more robust fix is to make sure the long listing you have is of the right form (typeset as links rather than code). I'll comment on the diff for the fix.

EDIT: never mind, there's already a special-casing of those constants, the issue was the listing in this PR is incomplete (e.g. constants.c went missing).

rgommers · 2020-03-19T16:10:30Z

This looks a little weird:

the first three lines shouldn't have line breaks between them I think.

rgommers · 2020-03-19T16:12:31Z

scipy/constants/__init__.py


-===========================  =================================================================
-``c``                        speed of light in vacuum


Actually, this you removed and didn't put back. Same for the other ones - that explains the CI failure

ilayn · 2020-03-20T20:45:11Z

@jakobjakobson13 Friendly nudge about online changes. You can still push your changes all at once at one go via git commits. Then you don't need to save things and trigger CI/CD pipelines with which we don't want to be throttled.

If you need help with offline builds please ask for help.

jakobjakobson13 · 2020-03-20T20:48:32Z

@jakobjakobson13 Friendly nudge about online changes. You can still push your changes all at once at one go via git commits. Then you don't need to save things and trigger CI/CD pipelines with which we don't want to be throttled.

If you need help with offline builds please ask for help.

Okay, got it.

jakobjakobson13 · 2020-04-03T08:52:10Z

Before you push this comment to the master branch, I have the following comments:

I also reduced to constants.py file by deleting all constants from that file. So this would break compatibility of scripts that used something like scipy.constants.c for the speed of light. Also the cleanup leaves only the auxiliary functions and so the filename gets misleading. I also deleted the prefixes even though I thought about putting them into a new file called prefixes.py.
As the seven constants for the seven base units should have fixed values for the next years, I wanted to test them by hard coding their values to the test module. My local tests failed however as I could not test as the constants hyperfine coupling constant of casium-133 could not be found.
I would find a function that simplifies the units of a calculation quite convenient. To implement this however, you would have to convert all derived units into SI units (that would be easy), convert them to sympy variables (quite easy), simplify the resulting term (easy) and possibly reconvert them into a SI derived unit (quite tricky). But it would definitely be handy.

pv · 2020-04-10T16:51:46Z

scipy/constants/__init__.py


+About the SI units


This is reference documentation, so background material would better go to the bottom of the page, as people are mainly going to be looking for variable/function names etc. (except maybe the first time when reading this).

Okay, I'll do it later.

pv · 2020-04-10T16:56:04Z

scipy/constants/constants.py

-# functions for conversions that are not linear
-
+from .codata import value
+import numpy as np


This is no-go, the constants in this file can't be just removed without breaking backward compat.

I'd recommend to stick with documentation changes in this PR, other unrelated changes should go to separate PRs.

This is no-go, the constants in this file can't be just removed without breaking backward compat.

Okay, I'll revert it later.

I'd recommend to stick with documentation changes in this PR, other unrelated changes should go to separate PRs.

Should I go for it anyway as it breaks backwards compatibility?

Should I go for it anyway as it breaks backwards compatibility?

I think not, as we can't remove them (and deprecation etc. does not seem warranted, and hard to do for constants). But as a general comment, it's better to stick to one topic in one PR.

jakobjakobson13 · 2020-07-21T17:11:33Z

As I deleted my scipy repository and the specific branch, I'm closing this pull request.

tupui · 2022-12-09T11:48:20Z

@jakobjakobson13 do you still plan to work on this?

jakobjakobson13 · 2022-12-09T17:24:32Z

@jakobjakobson13 do you still plan to work on this?

That´s up to you.

However, if I work on it, I´ll slim it down drastically with the main page consisting of the following information:

The seven SI defining constants of the SI [1]
The CODATA contants given in [2, 3] (as Scipy currently provides them).

And a subpage giving the following background:

The SI base units [1]
Base quantities and dimensions used in the SI [1]
SI prefixes [1]
The 22 SI units with special names and symbols [1]
Examples of coherent derived units in the SI expressed in terms of base
units [1]
Examples of SI coherent derived units whose names and symbols
include SI coherent derived units with special names and symbols [1]
Non-SI units accepted for use with the SI Units [1]

[1] The International System of Units – 9th edition – Complete text in English and French (2019)
[2] E. Tiesinga, P. J. Mohr, D. B. Newell, and B. N. Taylor, J. Phys. Chem. Ref. Data 50, 033105 (2021)
[3] E. Tiesinga, P. J. Mohr, D. B. Newell, and B. N. Taylor, Rev. Mod. Phys. 93, 025010 (2021)

jakobjakobson13 · 2022-12-09T17:28:46Z

Also, I deleted my fork on my hard drive and on Github. Does this mean, I would have to restart from scratch?

tupui · 2022-12-09T17:36:05Z

You seems to be the reference for this module so I would trust your judgment.

Also, I deleted my fork on my hard drive and on Github. Does this mean, I would have to restart from scratch?

No 😃 The changes are still available here and you can use for instance GitHub's CLI to pull it with gh pr checkout 11682

jakobjakobson13 · 2022-12-10T17:02:28Z

After second thoughts, I would defer this pull request to some time later. Feel free to take over.

Pretty drastic rewrite of __init__py

80df09b

If this rewrite is too drastic, let me know.

pv changed the title ~~Pretty drastic rewrite of constants init~~ DOC: drastic rewrite of constants documentation Mar 18, 2020

miladsade96 added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.constants labels Mar 18, 2020

jakobjakobson13 added 9 commits March 18, 2020 22:17

Cleaning up and corrections

2647c7d

fix of references and lists

9b167ea

further list fixes

c693761

fix references

286fc06

Further rewrites and formatting

a1ff9d8

minor grammatical fixes

16d1700

reduced number of references

ebe41a9

minor rewrites

10c827c

Move reference to end of sentence

39c6063

rgommers reviewed Mar 19, 2020

View reviewed changes

jakobjakobson13 added 2 commits March 19, 2020 12:39

symbols in brackets; rewritten dictionary sentence

41fd3d4

Change name of reference; reintroduce global data

96dc5fc

rgommers reviewed Mar 19, 2020

View reviewed changes

tylerjereddy mentioned this pull request Mar 20, 2020

TestDifferentialEvolutionSolver::test_L4 failing in CI #11693

Closed

jakobjakobson13 added 4 commits March 20, 2020 19:56

minor typos, reinclude auxilliary functions

f7a0158

drastic slimming down of constants.py

6385e30

fix variable lambda to lambda_

dec5237

simplify test_basic_table_parse

7f06356

jakobjakobson13 added 3 commits March 20, 2020 21:20

change speed_of_light to c

779c1f9

include phi or golden_ratio respectively

5f554bb

include phi or golden ratio respectively

bf269b7

Ich added 8 commits March 21, 2020 18:27

change _math.sqrt to np.sqrt

d82b0fa

fix doctest, pep8 warning and refguide

e03f21c

further fixes

1ab3967

move phi to func, fix tests

88c2cdb

fix docs by deleting object in codata

45cc0a3

cleanup codata docstring

dbde5e0

reintroduce dict to doc; simplify tests

c3a5985

revert file encoding to ASCII

06b8b58

pv requested changes Apr 10, 2020

View reviewed changes

Revert all changes except doc related ones

e5a139a

jakobjakobson13 closed this Jul 21, 2020

tupui mentioned this pull request Dec 9, 2022

ENH: Add units to scipy.constants description #17571

Open

jakobjakobson13 mentioned this pull request Dec 10, 2022

MAINT: constants: slim down module #17577

Draft

AlexSzatmary mentioned this pull request Feb 11, 2024

DOC: Add units to scipy.constants description #18979

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: drastic rewrite of constants documentation #11682

DOC: drastic rewrite of constants documentation #11682

jakobjakobson13 commented Mar 18, 2020

ilayn commented Mar 19, 2020

jakobjakobson13 commented Mar 19, 2020

rgommers Mar 19, 2020

rgommers Mar 19, 2020

rgommers commented Mar 19, 2020

rgommers Mar 19, 2020

jakobjakobson13 Mar 19, 2020 •

edited

rgommers Mar 19, 2020

rgommers Mar 19, 2020

jakobjakobson13 commented Mar 19, 2020

rgommers commented Mar 19, 2020 •

edited

rgommers commented Mar 19, 2020

rgommers Mar 19, 2020

ilayn commented Mar 20, 2020 •

edited

jakobjakobson13 commented Mar 20, 2020

jakobjakobson13 commented Apr 3, 2020

pv Apr 10, 2020

jakobjakobson13 Apr 10, 2020

pv Apr 10, 2020 •

edited

jakobjakobson13 Apr 10, 2020

pv Apr 10, 2020 •

edited

jakobjakobson13 commented Jul 21, 2020

tupui commented Dec 9, 2022

jakobjakobson13 commented Dec 9, 2022 •

edited

jakobjakobson13 commented Dec 9, 2022

tupui commented Dec 9, 2022

jakobjakobson13 commented Dec 10, 2022


		=========================== =================================================================
		``c`` speed of light in vacuum

DOC: drastic rewrite of constants documentation #11682

DOC: drastic rewrite of constants documentation #11682

Conversation

jakobjakobson13 commented Mar 18, 2020

Reference issue

What does this implement/fix?

Related pull requests

ilayn commented Mar 19, 2020

jakobjakobson13 commented Mar 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers commented Mar 19, 2020

Choose a reason for hiding this comment

jakobjakobson13 Mar 19, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakobjakobson13 commented Mar 19, 2020

rgommers commented Mar 19, 2020 • edited

rgommers commented Mar 19, 2020

Choose a reason for hiding this comment

ilayn commented Mar 20, 2020 • edited

jakobjakobson13 commented Mar 20, 2020

jakobjakobson13 commented Apr 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pv Apr 10, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pv Apr 10, 2020 • edited

Choose a reason for hiding this comment

jakobjakobson13 commented Jul 21, 2020

tupui commented Dec 9, 2022

jakobjakobson13 commented Dec 9, 2022 • edited

jakobjakobson13 commented Dec 9, 2022

tupui commented Dec 9, 2022

jakobjakobson13 commented Dec 10, 2022

jakobjakobson13 Mar 19, 2020 •

edited

rgommers commented Mar 19, 2020 •

edited

ilayn commented Mar 20, 2020 •

edited

pv Apr 10, 2020 •

edited

pv Apr 10, 2020 •

edited

jakobjakobson13 commented Dec 9, 2022 •

edited