Updated changes #79
Updated changes #79
Conversation
|
hi @bittremieux ,i have formatted the code as required by ruff ,but our workflows seems to be failing due to pyteomics issue only ,rest seems to be working fine . |
bittremieux
left a comment
bittremieux
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I have a few small suggestions and questions.
|
|
||
| # Reload the Pyteomics PROXI aggregator to also include GNPS. | ||
| pyteomics.usi._proxies["gnps"] = GnpsBackend | ||
| # Only perform the assignment if not building docs. |
There was a problem hiding this comment.
question: Why is this necessary?
There was a problem hiding this comment.
hi @bittremieux ,its good to see u again .so this ensures that the GNPS backend will be used for normal runtime usage, while avoiding potential issues during Sphinx documentation builds or in environments .so that while doing doctest it should not cause any issue .
docs/src/quickstart.md
Outdated
| plt.savefig("quickstart.png", bbox_inches="tight", dpi=300, transparent=True) | ||
| plt.close() | ||
| >>> fig, ax = plt.subplots(figsize=(12, 6)) | ||
| >>> _=sup.spectrum(spectrum, grid=False, ax=ax); |
There was a problem hiding this comment.
todo: Remove leading _= and trailing ;.
There was a problem hiding this comment.
Actually sup.spectrum(spectrum, grid=False, ax=ax) was returning a Axes object. Since our expected output is “nothing,” doctest flags this as a failure ,so to avoid this i have store that in '_' . i will remove this for the time being.
| clause.property_value.relation.prefix | ||
| == "monoIsotopicMass" | ||
| ) | ||
| and "monoIsotopicMass" |
There was a problem hiding this comment.
question: What is the advantage of the new version vs the previous code?
There was a problem hiding this comment.
well the main reason for this change because from the previous code we were facing attribute error ,as it was depending on how fastobo parses the OBO file, the relation might include an extra colon or whitespace (e.g. "monoIsotopicMass:" or with surrounding spaces) as seen in XLMOD.obo file. so i change the previous implementation so that minor formatting differences don’t cause the condition to fail.
| --- MsmsSpectrum.annotate_proforma(proforma_str =proforma_sequence, fragment_tol_mass=10.0, fragment_tol_mode ='ppm', ion_types="by") | ||
|
|
||
| or | ||
| --- or |
There was a problem hiding this comment.
todo: This interprets it as a Python or statement. Better to not have this be interpreted by doctest.
There was a problem hiding this comment.
okay i will update this also .but is not causing any issue + it will help in maintaining the identation with other code .
|
hi @bittremieux plz have a look at the new commit . |
|
@bittremieux ,if u have a moment then ,plz have a look at the new commit ;if there is some other thing that needs to be changed in the pr do let me know . |
|
hi @bittremieux ,if u have a moment then can u plz have a look at this pr . |
|
hey @bittremieux ,do i need to open a new pull request to avoid this merge conflict. |
2 participants
these are some of the changes to reduce errors occuring while testing + i have added the doctest for quicktest.md (rest are still pending ).