Crypto-TII / CryptographicEstimators

This project gathers and standardize command line scripts to estimate the difficulty of solving hard mathematical problems related to cryptography.
https://estimators.crypto.tii.ae/
GNU General Public License v3.0
32 stars 4 forks source link

Refactor Docstring Style to Google Style #162

Open Dioprz opened 1 month ago

Dioprz commented 1 month ago

Currently, our codebase uses a non-standard docstring style inspired by sage docstrings. While functional, this style presents several drawbacks, including poor support for automatic documentation generation tools like Sphinx and inconsistencies in formatting that includes:

This issue proposes migrating to the industry-standard Google docstring style to address these shortcomings while capitalizing on the ongoing migration from sage doctest to Python's native doctest.

Current Style:

"""
Construct an instance of crossbred estimator

The Crossbred is an algorithm to solve the MQ problem [JV18]_...
.. NOTE::

    Our complexity estimates are a generalization over any field of size `q` of the complexity formulas given in
    [Dua20]_, which are given either for `q=2` or generic fields.

INPUT:

- ``problem`` -- MQProblem object including all necessary parameters
- ``h`` -- external hybridization parameter (default: 0)
- ``w`` -- linear algebra constant (2 <= w <= 3) (default: 2.81)
- ``max_D`` -- upper bound to the parameter D (default: 20)
- ``memory_access`` -- specifies the memory access cost model (default: 0, choices: 0 - constant, 1 - logarithmic, 2 - square-root, 3 - cube-root or deploy custom function which takes as input the logarithm of the total memory usage)
- ``complexity_type`` -- complexity type to consider (0: estimate, 1: tilde O complexity, default: 0)

EXAMPLES::

    sage: from cryptographic_estimators.MQEstimator.MQAlgorithms.crossbred import Crossbred
    sage: from cryptographic_estimators.MQEstimator.mq_problem import MQProblem
    sage: E = Crossbred(MQProblem(n=10, m=12, q=5))
    sage: E
    Crossbred estimator for the MQ problem with 10 variables and 12 polynomials

"""

Proposed Style (Google Docstring):

  """Construct an instance of crossbred estimator.

  The Crossbred is an algorithm to solve the MQ problem [JV18]_... 

  Note:
    Our complexity estimates are a generalization over any field of size `q`
    of the complexity formulas given in [Dua20]_, which are given either for
    `q=2` or generic fields.

  Args:
    problem: MQProblem object including all necessary parameters.
    h: External hybridization parameter (default: 0).
    w: Linear algebra constant (2 <= w <= 3) (default: 2.81).
    max_D: Upper bound to the parameter D (default: 20).
    memory_access: Specifies the memory access cost model (default: 0, choices:
      0 - constant, 1 - logarithmic, 2 - square-root, 3 - cube-root or deploy
      custom function which takes as input the logarithm of the total memory usage).
    complexity_type: Complexity type to consider (0: estimate, 1: tilde O
      complexity, default: 0).

  Examples:
    >>> from cryptographic_estimators.MQEstimator.MQAlgorithms.crossbred import Crossbred
    >>> from cryptographic_estimators.MQEstimator.mq_problem import MQProblem
    >>> E = Crossbred(MQProblem(n=10, m=12, q=5))
    >>> E
    Crossbred estimator for the MQ problem with 10 variables and 12 polynomials
  """

Advantages of adopting this style include:

  1. Industry Standard: Ensures better compatibility with various documentation generation tools and IDEs.
  2. Improved Readability: Promotes cleaner, more concise, and visually appealing docstrings.
  3. Simplified Documentation Generation: Leverages robust Sphinx support through the Napoleon extension for effortless automatic documentation creation.
  4. Consistent Structure: Provides well-defined and extensible sections like Args:, Returns:, Raises:, and more, ensuring uniformity across the codebase.

Importantly, this migration will not introduce significant overhead. The changes can be largely automated using carefully supervised AI prompts for semi-automated docstring generation.

Feedback required: @Javierverbel @Memphisd @FloydZ @fmerino21

Memphisd commented 1 month ago

I agree with that. The usage of this style was without much thought, as you correctly pointed out it was just inspired by what sage uses. Will this be automatically recognized or does this require some modifications of the setup / configuration files?

Dioprz commented 1 month ago

Great!

I'm not sure what do you mean by modifications on the setup/configuration files... Could you please expand a bit on that? (Maybe the change requires it and I don't know!)