diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py
index b68b197..8091249 100644
--- a/cellmlmanip/units.py
+++ b/cellmlmanip/units.py
@@ -369,11 +369,11 @@ class UnitStore(object):
         logger.debug('evaluate_units(%s) ⟶ %s', expr, found)
         return found
 
-    def get_conversion_factor(self, to_unit, from_unit):
+    def get_conversion_factor(self, from_unit, to_unit):
         """Returns the magnitude multiplier required to convert a unit to the specified unit.
 
-        :param to_unit: Unit object into which the units should be converted
-        :param from_unit: the Unit to be converted
+        :param from_unit: the ``Unit`` to be converted
+        :param to_unit: ``Unit`` object into which the units should be converted
         :returns: the magnitude of the resulting conversion factor
         """
         assert isinstance(from_unit, self._registry.Unit), 'from_unit must be a unit, not ' + str(from_unit)
@@ -716,14 +716,14 @@ class UnitCalculator(object):
         was_converted = False  # Tracks whether we needed a units conversion
         dimensionless = self._store.get_unit('dimensionless')
 
-        def maybe_convert_expr(expr, was_converted, to_units, from_units):
+        def maybe_convert_expr(expr, was_converted, from_units, to_units):
             """Helper function that adds a conversion factor if needed.
 
             :returns: ``(expr or new expr, was_converted, to_units)``
             :raises UnitConversionError: if conversion impossible
             """
             try:
-                cf = self._store.get_conversion_factor(to_units, from_units)
+                cf = self._store.get_conversion_factor(from_units, to_units)
             except DimensionalityError:
                 raise UnitConversionError(expr, from_units, to_units) from None
             if cf != 1:
@@ -749,7 +749,7 @@ class UnitCalculator(object):
                 # Nothing to do, just record the actual units
                 actual_units = expr.units
             else:
-                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, to_units, expr.units)
+                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, expr.units, to_units)
         elif expr.is_Derivative:
             # Just convert the result if needed
             if (not isinstance(expr.args[0], model.VariableDummy) or
@@ -762,7 +762,7 @@ class UnitCalculator(object):
             _, was_converted, denominator_units = maybe_convert_child(expr.args[1][0], was_converted, None)
             actual_units = numerator_units / denominator_units
             if to_units is not None:
-                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, to_units, actual_units)
+                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, actual_units, to_units)
         elif expr.is_Mul:
             # Multiply units of operands, which are allowed to be of any units
             new_args = []
@@ -775,7 +775,7 @@ class UnitCalculator(object):
             if to_units is not None or was_converted:
                 # Convert the result if required
                 expr = expr.func(*new_args)
-                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, to_units, actual_units)
+                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, actual_units, to_units)
         elif expr.is_Pow:
             # Pow is used by Sympy for exponentiating, roots and division
             base, exponent = expr.args
@@ -791,7 +791,7 @@ class UnitCalculator(object):
                 expr = expr.func(base, exponent)
             actual_units = base_units ** exponent_val
             if to_units is not None:
-                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, to_units, actual_units)
+                expr, was_converted, actual_units = maybe_convert_expr(expr, was_converted, actual_units, to_units)
         elif expr.is_Add:
             # Convert all arguments to the desired units
             new_args = []

Use consistent argument ordering for unit conversions?
Currently we have a mix of `to_units, from_units` (e.g. `UnitStore.get_conversion_factor`) and `from_units, to_units` (e.g. `UnitStore.convert`). We should pick one!

@MichaelClerx and @jonc125 prefer `from, to`. However the main method used by external code is probably `get_conversion_factor` so this change would impact @MauriceHendrix .

Should we make the change?
diff --git a/tests/test_units.py b/tests/test_units.py
index 185e50e..87545a5 100644
--- a/tests/test_units.py
+++ b/tests/test_units.py
@@ -149,8 +149,8 @@ class TestUnits:
         store = UnitStore()
         s = store.get_unit('second')
         ms = store.add_unit('ms', 'second / 1000')
-        assert store.get_conversion_factor(s, ms) == 0.001
-        assert store.get_conversion_factor(ms, s) == 1000
+        assert store.get_conversion_factor(ms, s) == 0.001
+        assert store.get_conversion_factor(s, ms) == 1000
 
     def test_prefixing(self):
         """Tests edge cases for the internal prefixing."""
@@ -203,8 +203,8 @@ class TestUnits:
         with pytest.raises(KeyError):
             b.get_unit('x')
 
-        assert a.get_conversion_factor(x, y) == 0.001
-        assert b.get_conversion_factor(x, y) == 0.001
+        assert a.get_conversion_factor(x, y) == 1000
+        assert b.get_conversion_factor(x, y) == 1000
 
 
 class TestEvaluateUnits:

pip install -e .[dev]
parse_log_pytest
requirements.txt
pytest pytest-cov flake8 isort codecov
3.6
requirements/base.txt
pytest --no-header -rA --tb=line --color=no -p no:cacheprovider -W ignore::DeprecationWarning
attrs==22.2.0
-e git+https://github.com/ModellingWebLab/cellmlmanip.git@681131c93572c6685c694b8f371dac0558ba4c6b#egg=cellmlmanip
certifi==2021.5.30
charset-normalizer==2.0.12
codecov==2.1.13
coverage==6.2
decorator==4.4.0
flake8==5.0.4
idna==3.10
importlib-metadata==4.2.0
iniconfig==1.1.1
isodate==0.6.0
isort==5.10.1
lxml==4.4.1
mccabe==0.7.0
mpmath==1.1.0
networkx==2.3
packaging==21.3
Pint==0.9
pluggy==1.0.0
py==1.11.0
pycodestyle==2.9.1
pyflakes==2.5.0
pyparsing==2.4.2
pytest==7.0.1
pytest-cov==4.0.0
rdflib==4.2.2
requests==2.27.1
six==1.12.0
sympy==1.4
tomli==1.2.3
typing_extensions==4.1.1
urllib3==1.26.20
zipp==3.6.0

name: cellmlmanip
channels:
  - defaults
  - https://repo.anaconda.com/pkgs/main
  - https://repo.anaconda.com/pkgs/r
  - conda-forge
dependencies:
  - _libgcc_mutex=0.1=main
  - _openmp_mutex=5.1=1_gnu
  - ca-certificates=2025.2.25=h06a4308_0
  - certifi=2021.5.30=py36h06a4308_0
  - ld_impl_linux-64=2.40=h12ee557_0
  - libffi=3.3=he6710b0_2
  - libgcc-ng=11.2.0=h1234567_1
  - libgomp=11.2.0=h1234567_1
  - libstdcxx-ng=11.2.0=h1234567_1
  - ncurses=6.4=h6a678d5_0
  - openssl=1.1.1w=h7f8727e_0
  - pip=21.2.2=py36h06a4308_0
  - python=3.6.13=h12debd9_1
  - readline=8.2=h5eee18b_0
  - setuptools=58.0.4=py36h06a4308_0
  - sqlite=3.45.3=h5eee18b_0
  - tk=8.6.14=h39e8969_0
  - wheel=0.37.1=pyhd3eb1b0_0
  - xz=5.6.4=h5eee18b_1
  - zlib=1.2.13=h5eee18b_1
  - pip:
      - attrs==22.2.0
      - charset-normalizer==2.0.12
      - codecov==2.1.13
      - coverage==6.2
      - decorator==4.4.0
      - flake8==5.0.4
      - idna==3.10
      - importlib-metadata==4.2.0
      - iniconfig==1.1.1
      - isodate==0.6.0
      - isort==5.10.1
      - lxml==4.4.1
      - mccabe==0.7.0
      - mpmath==1.1.0
      - networkx==2.3
      - packaging==21.3
      - pint==0.9
      - pluggy==1.0.0
      - py==1.11.0
      - pycodestyle==2.9.1
      - pyflakes==2.5.0
      - pyparsing==2.4.2
      - pytest==7.0.1
      - pytest-cov==4.0.0
      - rdflib==4.2.2
      - requests==2.27.1
      - six==1.12.0
      - sympy==1.4
      - tomli==1.2.3
      - typing-extensions==4.1.1
      - urllib3==1.26.20
      - zipp==3.6.0
prefix: /opt/conda/envs/cellmlmanip

tests/test_units.py::TestUnits::test_get_conversion_factor tests/test_units.py::TestUnits::test_shared_registry
tests/test_units.py::TestUnits::test_add_unit tests/test_units.py::TestUnits::test_add_base_unit tests/test_units.py::TestUnits::test_convert tests/test_units.py::TestUnits::test_format tests/test_units.py::TestUnits::test_get_unit tests/test_units.py::TestUnits::test_is_equivalent tests/test_units.py::TestUnits::test_prefixing tests/test_units.py::TestEvaluateUnits::test_numbers tests/test_units.py::TestEvaluateUnits::test_others tests/test_units.py::TestConvertingExpressions::test_variable_no_conversion tests/test_units.py::TestConvertingExpressions::test_variable_conversion tests/test_units.py::TestConvertingExpressions::test_number_conversion tests/test_units.py::TestConvertingExpressions::test_plain_numbers tests/test_units.py::TestConvertingExpressions::test_derivative_no_conversion tests/test_units.py::TestConvertingExpressions::test_derivative_conversion tests/test_units.py::TestConvertingExpressions::test_mul_and_pow tests/test_units.py::TestConvertingExpressions::test_square_root tests/test_units.py::TestConvertingExpressions::test_add_and_subtract tests/test_units.py::TestConvertingExpressions::test_abs_ceil_floor tests/test_units.py::TestConvertingExpressions::test_exp_log_trig tests/test_units.py::TestConvertingExpressions::test_relations tests/test_units.py::TestConvertingExpressions::test_piecewise tests/test_units.py::TestConvertingExpressions::test_assignment tests/test_units.py::TestConvertingExpressions::test_set_lhs_units_from_rhs tests/test_units.py::TestConvertingExpressions::test_set_lhs_units_from_converted_rhs tests/test_units.py::TestConvertingExpressions::test_symbol_wrong_dimensions tests/test_units.py::TestConvertingExpressions::test_derivative_wrong_dimensions tests/test_units.py::TestConvertingExpressions::test_complex_derivative tests/test_units.py::TestConvertingExpressions::test_mul_wrong_dimensions tests/test_units.py::TestConvertingExpressions::test_error_matrix tests/test_units.py::TestConvertingExpressions::test_error_exponent_not_dimensionless tests/test_units.py::TestConvertingExpressions::test_error_exponent_not_number tests/test_units.py::TestConvertingExpressions::test_pow_wrong_dimensions tests/test_units.py::TestConvertingExpressions::test_add_wrong_dimensions tests/test_units.py::TestConvertingExpressions::test_relational_must_be_dimensionless tests/test_units.py::TestConvertingExpressions::test_relational_dimension_mismatch tests/test_units.py::TestConvertingExpressions::test_piecewise_condition_not_dimensionless tests/test_units.py::TestConvertingExpressions::test_piecewise_cannot_convert_result tests/test_units.py::TestConvertingExpressions::test_exp_must_be_dimensionless tests/test_units.py::TestConvertingExpressions::test_number_must_be_dimensionless tests/test_units.py::TestConvertingExpressions::test_error_unsupported_math tests/test_units.py::TestConvertingExpressions::test_set_lhs_units_from_inconsistent_rhs
cellmlmanip/units.py
cellmlmanip/units.py