Skip to content

Conversation

@nawtrey
Copy link
Collaborator

@nawtrey nawtrey commented Apr 20, 2025

Description

  • Deprecates the parameter dirpar_edges in the following functions:

    • calculations.calc_sigma
    • calculations.calc_state_probs_from_diags
    • calculations.calc_net_cycle_flux_from_diags
  • Re-introduces (with deprecated status) the calculations.calc_net_cycle_flux to allow
    for KDA paper code to continue working

  • Updates docstrings for aforementioned functions so it is clear under which circumstances the
    deprecation warnings will be produced

  • Updates Test_Diagram_Generation.test_diagram_counts and Test_Diagram_Generation.test_max_connected_diagram_counts to remove mentions of dirpars

  • Updates to test_function_inputs:

    • Adds 3 test cases to confirm DeprecationWarnings are triggered when dirpar_edges is used in calc_sigma, calc_state_probs_from_diags, etc.
    • Adds 2 test cases to confirm a TypeError is raised when no directional diagram edges/diagrams are input
    • Adds 2 test cases to confirm multiple DeprecationWarnings are raised when calling calc_state_probs_from_diags or calc_net_cycle_flux_from_diags with the deprecated parameter dirpar_edges

Status

  • Ready to go

* Deprecates the parameter `dirpar_edges` in
the following functions:
  - `calculations.calc_sigma`
  - `calculations.calc_state_probs_from_diags`
  - `calculations.calc_net_cycle_flux_from_diags`

* Re-introduces (with deprecated status) the
`calculations.calc_net_cycle_flux` to allow
for KDA paper code to continue working

* Updates docstrings for aforementioned functions
so it is clear under which circumstances the
deprecation warnings will be produced

* Updates `Test_Diagram_Generation.test_diagram_counts` and
`Test_Diagram_Generation.test_max_connected_diagram_counts`
to remove mentions of `dirpars`

* Updates to `test_function_inputs`:
  - Adds 3 test cases to confirm `DeprecationWarning`s are
    triggered when `dirpar_edges` is used in `calc_sigma`,
    `calc_state_probs_from_diags`, etc.
  - Adds 2 test cases to confirm a `TypeError` is raised
    when no directional diagram edges/diagrams are input
  - Adds 2 test cases to confirm multiple
    `DeprecationWarning`s are raised when calling
    `calc_state_probs_from_diags` or
    `calc_net_cycle_flux_from_diags` with the
    deprecated parameter `dirpar_edges`
@nawtrey
Copy link
Collaborator Author

nawtrey commented Apr 20, 2025

One of the new test cases failed --

        with pytest.warns(DeprecationWarning) as record:
            # call using deprecated parameter, `dirpar_edges`
            calculations.calc_net_cycle_flux_from_diags(
                G, cycle=cycle, dirpar_edges=dir_edges, order=order,
                key="val", output_strings=False,
            )
            # check that both DeprecationWarnings are raised
            dep_msg_1 = record[0].message.args[0]
            dep_msg_2 = record[1].message.args[0]
>           assert len(record) == 2
E           assert 4 == 2
E            +  where 4 = len(WarningsChecker(record=True))

I looked into it locally, and it turns out this is related to pandas somehow. If I uninstall pandas and print all 4 warnings, this is what I get:

kda\tests\test_kda.py record 1 The `dirpar_edges` parameter is deprecated and will be removed in a future release. Please use `dir_edges` instead.
record 2 `kda.calculations.calc_net_cycle_flux_from_diags` is deprecatedand will be removed in a future release. Use `kda.calculations.calc_cycle_flux` with parameter `dir_edges`.
record 3 pandas not found, skipping conversion test.
record 4 pandas not found, skipping conversion test.

If I install pandas, warnings 3 and 4 go away.

On further inspection it looks like its coming from networkx (see here). The assertion regarding the number of records is not important, so I think I'll just drop it.

* Removes check for new `DeprecationWarning`
assertion related to the number of warnings
given when `calc_net_cycle_flux_from_diags`
is called.
@nawtrey
Copy link
Collaborator Author

nawtrey commented Apr 20, 2025

Alright, all tests are passing, docs look good, and the KDA paper code still works on this branch (primarily due to the re-introduction of calc_net_cycle_flux). The code coverage looks good. Merging.

@nawtrey nawtrey merged commit f7bbaf8 into master Apr 20, 2025
6 checks passed
@nawtrey nawtrey deleted the fix_dirpar_instances branch April 20, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants