Refactor/io sbml

[akaviaLab]

• description of feature/fix
  Updated sbml.py and test_sbml.py to Python 3.6+ including removing future, f-strings, typing. I also made the typing comments in sbml.py to typing annotations.

[codecov-commenter]

Codecov Report

Merging #1182 (1b0eca1) into devel (7d95a4a) will increase coverage by 0.00%.
The diff coverage is 88.88%.

❗ Current head 1b0eca1 differs from pull request most recent head 2914f57. Consider uploading reports for the commit 2914f57 to get more accurate results

@@           Coverage Diff           @@
##            devel    #1182   +/-   ##
=======================================
  Coverage   83.90%   83.91%           
=======================================
  Files          65       65           
  Lines        5363     5365    +2     
  Branches     1241     1241           
=======================================
+ Hits         4500     4502    +2     
  Misses        553      553           
  Partials      310      310           

Impacted Files	Coverage Δ
src/cobra/io/sbml.py	80.11% <88.67%> (ø)
src/cobra/core/gene.py	79.50% <100.00%> (+0.14%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d95a4a...2914f57. Read the comment docs.

[akaviaLab]

One question, possibly for @cdiener - Why do we have clip(), and not use something like replace(sid, prefix, 1) or regex?

[cdiener]

Looks good. Just a typo.

[akaviaLab]

Fixed typo. Still wondering why we use the function _clip(sid: str, prefix: str) -> str: and not something like replace(sid, prefix, 1). Mostly curious.

[cdiener]

Fixed typo. Still wondering why we use the function _clip(sid: str, prefix: str) -> str: and not something like replace(sid, prefix, 1). Mostly curious.

Because, str.replace replaces anywhere in the string but we only want to replace at the beginning of the string here. And str.lstrip only works with single characters.

[synchon]

The second parameter.

[synchon]

A few comments. Thank you for the work!

[synchon]

Although a lot a documentation comments, but it is just to help us move faster to enabling flake8 check. A general comment: please try to maintain the nummpydoc documentation conventions.

You have also put a lot of type annotations on variables in the module scope, not really sure if we are doing that. Maybe others can vote for the decision.

[akaviaLab]

Although a lot a documentation comments, but it is just to help us move faster to enabling flake8 check. A general comment: please try to maintain the nummpydoc documentation conventions.

You have also put a lot of type annotations on variables in the module scope, not really sure if we are doing that. Maybe others can vote for the decision.

Originally, there were a lot of type annotation comments in this file. Annotation comments are Python 2 compliant, so I updated them to Python 3.

[synchon]

Although a lot a documentation comments, but it is just to help us move faster to enabling flake8 check. A general comment: please try to maintain the nummpydoc documentation conventions.
You have also put a lot of type annotations on variables in the module scope, not really sure if we are doing that. Maybe others can vote for the decision.

Originally, there were a lot of type annotation comments in this file. Annotation comments are Python 2 compliant, so I updated them to Python 3.

Yeah I understood why you did those but I don't know if we want those converted now, so left it open for suggestions.

[synchon]

@akaviaLab Let's keep the variable type annotation for now. If needed, we can remove it the future.

[synchon]

I'm okay with the changes. @cdiener If it's okay with you as well, we can merge the PR.

[cdiener]

👍