Skip to content

DOCSP-13855-mod-has-inconsistent-rounding #5587

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

Conversation

ianf-mongodb
Copy link
Contributor

No description provided.


The ``modulo`` expression rounds down decimal input towards zero.

The following examples demonstrates this behavior :
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[typo]

This should be "The following examples demonstrate..."

We also have an extra space between behavior and the colon.


The following examples demonstrates this behavior :

.. code-block:: javascript
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These examples are good, but this is not the most helpful format for users. Since this entire code block is copyable, users will expect to be able to copy and paste this block into their shell and have it run successfully. However, by including the input and output in the same code block, we run into issues.

One approach we could consider:

Place the input code block, followed by output code block, and label each code block accordingly. For example:

Input query:

.. code-block:: javascript

   db.inventory.find( { qty: { $mod: [ 4.0, 0 ] } } )

Results:

   .. code-block:: javascript
      :copyable: false
      
      { _id: 1, item: 'abc123', qty: 0 }
      { _id: 3, item: 'ijk123', qty: 12 }

And repeat this structure for each example.

{ _id: 1, item: 'abc123', qty: 0 }
{ _id: 3, item: 'ijk123', qty: 12 }

Each query applies 4 to the mod function regardless of decimal points
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since "4" is referring to a value in the code, we should monospace it:

``4``

Comment on lines 121 to 124
Modulo with Decimal Divisors
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The ``modulo`` expression rounds down decimal input towards zero.
Copy link
Collaborator

@jeff-allen-mongo jeff-allen-mongo Jul 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should refer to this expression as $mod (not Modulo). As currently written, it's reading like we're introducing a new concept because up until now, we've been referring to this expression exclusively as $mod.

{ _id: 3, item: 'ijk123', qty: 12 }

Each query applies 4 to the mod function regardless of decimal points
resulting in the same result set.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence was a little unclear to me and reads like it might be missing a comma. Consider something like:

Each of the example queries apply ``4`` to the mod function regardless of decimal points, and result in the same output.

Comment on lines 121 to 124
Modulo with Decimal Divisors
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The ``modulo`` expression rounds down decimal input towards zero.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the word "down"? If I have a negative number, rounding toward zero would be rounding up, not rounding down.

@@ -117,3 +117,31 @@ operator with an array that contains four elements:
"$err" : "bad query: BadValue malformed mod, too many elements",
"code" : 16810
}

Modulo with Decimal Divisors
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the previous review, Nikita suggested changing this section title to "Floating Point Arguments". Did we want to incorporate that suggestion?

@ianf-mongodb ianf-mongodb force-pushed the DOCS-13855-mod-has-inconsistent-rounding branch 12 times, most recently from c833ff9 to 67081b1 Compare July 20, 2021 20:04
@ianf-mongodb ianf-mongodb force-pushed the DOCS-13855-mod-has-inconsistent-rounding branch from 67081b1 to 08ac542 Compare July 20, 2021 20:20
@jeff-allen-mongo jeff-allen-mongo merged commit 992377d into mongodb:master Jul 20, 2021
mongo-cr-bot pushed a commit that referenced this pull request Dec 13, 2023
* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

---------

Co-authored-by: jason-price-mongodb <[email protected]>
mongo-cr-bot pushed a commit that referenced this pull request Dec 13, 2023
* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

---------

Co-authored-by: jason-price-mongodb <[email protected]>
mongo-cr-bot pushed a commit that referenced this pull request Dec 13, 2023
* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

---------

Co-authored-by: jason-price-mongodb <[email protected]>
mongo-cr-bot pushed a commit that referenced this pull request Dec 13, 2023
* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

---------

Co-authored-by: jason-price-mongodb <[email protected]>
mongo-cr-bot pushed a commit that referenced this pull request Dec 18, 2023
* Add useAuthorizationClaim

* fix formatting and add version

* one last formatting tweak

* review suggestions

* DOCS-16389 query plan cache subdocument (#5217)

* DOCS-16389 query plan cache subdocument

* *

* IF feedback

* clarity

* nit

* move to compatibility

* *

* wording

* wording

* (DOCS-16390) You can't specify WT encryption options in createCollection (#5260)

* (DOCS-16390) You can't specify WT encryption options in createCollection

* Includes change from tech review

* DOCS-16363 Document killedDueToMaxTimeMSExpired under serverStatus (#5193)

* DOCS-16363 Document killedDueToMaxTimeMSExpired under serverStatus

* add to TOC

* wording

* build error

* (DOCSP-34349) Adds release notes and compatibility changes for 7.2 (#5242)

* DOCS-16363 Document killedDueToMaxTimeMSExpired under serverStatus (#5193)

* DOCS-16363 Document killedDueToMaxTimeMSExpired under serverStatus

* add to TOC

* wording

* build error

* Includes PM external review changes

* Revert "Includes PM external review changes"

This reverts commit 0263ebe646848bf840c75b414815d3daa660e9fc.

* Includes changes from PM review

* Includes copy review changes

---------

Co-authored-by: Alison Huh <[email protected]>

* rewording

* staging to check formatting

* cleaning up the process

* formatting foo

* more review rewording and table formatting

* still trying to get the table format correct...

* funwith table formatting...

* table formatting...

* backport of docsp-16487 to 7.0

* fix table formatting and remove 7.2 release notes

* fix missing (*Also available in 7.0.5*) in previous PR

* review changes

* Update source/includes/fact-encryption-options-create-collection.rst

Co-authored-by: Alison Huh <[email protected]>

* DOCSP-34855-QE-update (#5587) (#5601)

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

* DOCSP-34855-QE-update

---------

Co-authored-by: jason-price-mongodb <[email protected]>

* DOCSP-35035 adding reference to large oplog section (#5598) (#5608)

* DOCSP-34820 4.4.27 Release notes (#5574) (#5590)

* DOCSP-35003-glossary-1 (#5596) (#5613)

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

* DOCSP-35003-glossary-1

---------

Co-authored-by: jason-price-mongodb <[email protected]>

* DOCS-14316-zones (#5551) (#5623)

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

* DOCS-14316-zones

---------

Co-authored-by: jason-price-mongodb <[email protected]>

* DOCSP-35004-glossary-2 (#5602) (#5629)

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

* DOCSP-35004-glossary-2

---------

Co-authored-by: jason-price-mongodb <[email protected]>

* fix backticks

* fix :: preceding code block

---------

Co-authored-by: Alison Huh <[email protected]>
Co-authored-by: Sarah Simpers <[email protected]>
Co-authored-by: jason-price-mongodb <[email protected]>
Co-authored-by: jason-price-mongodb <[email protected]>
Co-authored-by: ltran-mdb2 <[email protected]>
Co-authored-by: Kenneth P. J. Dyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants