Skip to content
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

ENH, API: Remove "key" and "name" parameters #84

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

nawtrey
Copy link
Collaborator

@nawtrey nawtrey commented Jun 4, 2024

Description

  • Remove the key and names parameters from
    all functions in calculations.py. Edge
    weights are now stored under a standard
    attribute name weight. For functions
    requiring the variable names (i.e. "k12") the
    strings are simply created on the fly using
    the available edge data.

  • Remove checks in calculations.py functions
    for key/output_strings mismatches as well
    as the test, test_kda.test_function_inputs, which
    checked TypeErrors were raised at appropriate
    times

  • Remove names, val_key, name_key parameters from
    graph_utils.generate_edges, as well as the associate test
    test_generate_edges_errors which checked the input
    parameters raised TypeErrors at appropriate times

  • Remove graph_utils.generate_K_string_matrix
    since it is no longer necessary for generating
    kinetic diagrams

  • Update documentation for all functions
    to reflect parameter changes

  • Fix diagrams.enumerate_partial_diagrams to build
    the adjacency matrix without retrieving the edge
    weights from the diagram

  • Update graph_utils.retrieve_rate_matrix
    to use NetworkX.to_numpy_array

  • Update function calls in test_kda.py
    to reflect parameter changes

  • Fix issue ENH: Change graphs to store rates as edge weights instead of attributes #56

Status

  • Ready to go

@nawtrey nawtrey added enhancement New feature or request API API-related changes labels Jun 4, 2024
@nawtrey
Copy link
Collaborator Author

nawtrey commented Jun 5, 2024

The codecov report failed to run. Looking at the logs it looks like it failed due to hitting the "rate limit":

{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 723s.', code='throttled')}{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 721s.', code='throttled')}{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 719s.', code='throttled')}{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 717s.', code='throttled')}{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 715s.', code='throttled')}{'detail': ErrorDetail(string='Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 712s.', code='throttled')}

It looks like this can be fixed by adding the codecov token to the repo and updating the YAML to snag it (see this comment and the codecov documentation).

I've already added the secret so I'll go ahead and open a new PR to update the YAML.

@nawtrey nawtrey mentioned this pull request Jun 5, 2024
1 task
* Remove the "key" and "name" parameters from
all functions in `calculations.py`. Edge
weights are now stored under a standard
attribute name `weight`. For functions
requiring the variable names (i.e. "k12") the
strings are simply created on the fly using
the available edge data.

* Update `graph_utils.retrieve_rate_matrix`
to use `NetworkX.to_numpy_array`.

* Update function calls in `test_kda.py` to
reflect parameter changes

* Fix `diagrams.enumerate_partial_diagrams`
to build the adjacency matrix without
retrieving the edge weights from the diagram

* Fix issue #56
@nawtrey nawtrey force-pushed the fix_issue_56_edge_weight_storage branch from 489a8e9 to 85f1100 Compare June 6, 2024 03:07
@nawtrey
Copy link
Collaborator Author

nawtrey commented Jun 6, 2024

Okay, now that codecov is back online I can confirm the function graph_utils.generate_K_string_matrix is completely uncovered. I think this can now be removed since we no longer need to worry about creating the kij's as strings.

* Remove `graph_utils.generate_K_string_matrix`
since it is no longer necessary for generating
kinetic diagrams
@nawtrey
Copy link
Collaborator Author

nawtrey commented Jun 6, 2024

The only thing left to do would be to update the root README, but that'll get completely rewritten in #83 so I'll leave that for now.

@nawtrey nawtrey merged commit 8515012 into master Jun 6, 2024
5 checks passed
@nawtrey nawtrey deleted the fix_issue_56_edge_weight_storage branch June 6, 2024 03:44
nawtrey added a commit that referenced this pull request Jul 12, 2024
nawtrey added a commit that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API-related changes enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant