Skip to content

Conversation

@sanjayankur31
Copy link
Member

Documents and adds type annotations to all functions in pynml.py. Now that we do have rtd set up, the documentation for these was lacking.

BREAKING CHANGE: changes the signature of the function, but only removes
a default argument that wasn't much used anyway.
BREAKING CHANGE: changes function signature, but the argument was a
default one and rarely used.
@sanjayankur31 sanjayankur31 added T: enhancement Type: enhancement S: ready for review Status: ready for review labels Jul 7, 2021
@sanjayankur31 sanjayankur31 requested a review from pgleeson July 7, 2021 15:14
@sanjayankur31 sanjayankur31 self-assigned this Jul 7, 2021
@sanjayankur31 sanjayankur31 added the C: docs Documentation related label Jul 7, 2021
@sanjayankur31
Copy link
Member Author

@pgleeson : this includes two minor changes to function signatures---I've removed the verbose arguments where it wasn't needed. So technically, this is a breaking change:

commit cb3a7e7f504a869ce1929948b818f0ae9e87440d
Author: Ankur Sinha (Ankur Sinha Gmail) <sanjay.ankur@gmail.com>
Date:   Wed Jul 7 11:55:36 2021 +0100

    refactor(pynml): remove unused default argument
    
    BREAKING CHANGE: changes function signature, but the argument was a
    default one and rarely used.

diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py
index b0e7a4d..50952e1 100644
--- a/pyneuroml/pynml.py
+++ b/pyneuroml/pynml.py
@@ -390,13 +390,12 @@ def convert_to_units(nml2_quantity, unit):
     return new_value
 
 
-def generate_nmlgraph(nml2_file_name, level=1, engine='dot', verbose_generate=True):
+def generate_nmlgraph(nml2_file_name, level=1, engine='dot'):
     """Generate NeuroML graph.
 
     :nml2_file_name (string): NML file to parse
     :level (string): level of graph to generate (default: '1')
     :engine (string): graph engine to use (default: 'dot')
-    :verbose_generate (boolean): output verbosity
 
     """
     from neuromllite.GraphVizHandler import GraphVizHandler

commit 48e160055f24a46c8f44153eebfdfd2b531ea4a9
Author: Ankur Sinha (Ankur Sinha Gmail) <sanjay.ankur@gmail.com>
Date:   Wed Jul 7 11:51:19 2021 +0100

    refactor(pynml): removes unneeded verbose argument
    
    BREAKING CHANGE: changes the signature of the function, but only removes
    a default argument that wasn't much used anyway.

diff --git a/pyneuroml/pynml.py b/pyneuroml/pynml.py
index de152d7..3be8c78 100644
--- a/pyneuroml/pynml.py
+++ b/pyneuroml/pynml.py
@@ -357,8 +357,8 @@ def get_value_in_si(nml2_quantity):
         return si_value
 
 
-def convert_to_units(nml2_quantity, unit, verbose=DEFAULTS['v']):
-    # type: (str, str, bool) -> float
+def convert_to_units(nml2_quantity, unit):
+    # type: (str, str) -> float
     """Convert a NeuroML2 quantity to provided unit.
 
     :param nml2_quantity: NeuroML2 quantity to convert
@@ -382,11 +382,10 @@ def convert_to_units(nml2_quantity, unit, verbose=DEFAULTS['v']):
             new_value = si_value / (un.scale * pow(10, un.power)) - un.offset
             if not un.dimension == dim:
                 raise Exception(
-                    "Cannot convert {} to {}. Dimensions of units ({}/{}) do not match!".format(
-                        nml2_quantity, unit, dim, un.dimension))
+                    "Cannot convert {} to {}. Dimensions of units ({}/{}) do not match!".format(nml2_quantity, unit, dim, un.dimension))
 
-    logger.debug("Converting {} {} to {}: {} ({} in SI units)".format(
-        m, u, unit, new_value, si_value), verbose)
+    logger.info("Converting {} {} to {}: {} ({} in SI units)".format(
+        m, u, unit, new_value, si_value))
 
     return new_value
 

@sanjayankur31 sanjayankur31 merged commit 34316d2 into development Jul 8, 2021
@sanjayankur31 sanjayankur31 deleted the feat/document-pynml branch July 19, 2021 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C: docs Documentation related S: ready for review Status: ready for review T: enhancement Type: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants