feat(parameters): add option to 3-1 cross-links #249
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #249 +/- ##
==========================================
- Coverage 69.37% 69.29% -0.08%
==========================================
Files 11 11
Lines 493 495 +2
==========================================
+ Hits 342 343 +1
- Misses 151 152 +1 ☔ View full report in Codecov by Sentry. |
TheLostLambda
force-pushed
the
mainline-3-1-crosslinks
branch
from
7da06a2
to
aa05a25
Compare
|
This is ready for review @ns-rse / @bobturneruk ! In the future I wonder if we do go back to committing the Perhaps the solution in the future is to hard-code in using the pre-release version in the PR? Then immediately commit to main and release the non-rc version after merging? Bit of a messy process at the moment! |
| @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
|
|
|||
| ## [Unreleased] | |||
|
|
|||
| ## [1.1.0] - 2023-10-15 | |||
There was a problem hiding this comment.
To avoid the circularity of having to release I would typically not detailed the changes in the CHANGELOG.md until after a release has been made.
We have automatic releasing to PyPI set up through a GitHub Action (.github/workflows/release-to-pypi.yaml) such that when a release is made, which obliges a tag to be added to the current HEAD commit, it triggers building and publishing.
There is also .github/workflows/release-to-orda.yaml which is triggered releases and tagging commits which uploads the artefacts to Sheffields ORDA figshare site (although it often times out and needs re-running).
I'll see if there is a way of automatically building the CHANGELOG.md from Release Notes in some manner (maybe there is an action that can be included).
There was a problem hiding this comment.
I think adding changes to the changelog under the "Unreleased" section seems like a nice thing to do in a PR, but I'll agree that pushing a release like this in a PR might be a bit much!
| @@ -171,6 +171,14 @@ def theo_masses_reader(file: Union[str, Path]) -> pd.DataFrame: | |||
| "columns. Have you checked the format of your database against one of the built-in databases?" | |||
| ) | |||
| ) from e | |||
| # Check that all structures are followed by "|n" where n is one or more digits | |||
| if not theo_masses_df["Inferred structure"].str.contains(r"\|\d+$").all(): | |||
| raise UserError( | |||
There was a problem hiding this comment.
Codecov is saying this eventuality isn't tested.
Should be relatively straight-forward to construct a test for this (and other checks if they're not already covered) by having a simple CSV without the |[1|2] construct and using the...
@pytest.mark.parameterize(
"file,error",
[
("tests/resources/file1.csv", UserError),
("tests/resources/file2.csv", ValueError),
]
)
def test_exceptions(file: str, error) -> None:
with pytest.raises(error)
assert theo_masses_reader(file)(or similar, bit of a rough hack/guess at the code).
There was a problem hiding this comment.
It's true that none of the exceptions are currently tested, I think, so I'll try to add in some proper tests in a bit!
There was a problem hiding this comment.
Sorry for the long silence @ns-rse ! How would you feel about writing up a Github issue for writing these tests (there are a lot of them and I might not have the time for a little while) and merging this PR so that the e2e tests are fixed for things like dependency bumping?
There was a problem hiding this comment.
(To clarify, I would write up the Github issue!)
There was a problem hiding this comment.
I'm of the opinion, based on my own failings in the past, that tests should be written when features are implemented. They purpose is after all to ensure that the code we write functions as expected and we ensure that by writing tests. In the past I've found that I let things slip or forget to update tests.
I've created the issue to remind us of this need in #251 mainly because I've started using the excellent browser extension Refined Github that simplifies the GitHub interface and adds useful features, one of which is an additional item to the ... of each message in PRs to Reference in new issue which takes the text of the post and creates a new issue out of it, including a link back to the original discussion at the bottom. Brilliant extension with a huge amount of tweaks which I'm only just starting to familiarise myself with.
No description provided.