Conversation
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
krzywon
left a comment
krzywon
left a comment
There was a problem hiding this comment.
A couple more comments.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
DrPaulSharp
left a comment
DrPaulSharp
left a comment
There was a problem hiding this comment.
A couple of things from me alongside Jeff's comments.
DrPaulSharp
force-pushed
the
refactor_24
branch
from
ee6af02 to
15acac0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The tests now properly appear in the pytest list of tests. Additionally, the tests are given readable parameterised names, but display the actual values on failure.
This is clearer than ArbitraryUnit, which might be confused for uncalibrated scattering data.
Current the only invalid characters are Space, /, and ^. I've also refactored the argument parsing to remove duplication between the numerator and denominator.
rprospero
force-pushed
the
172_unrecognized_units
branch
from
01deb22 to
7f9de96
Compare
There was a problem hiding this comment.
No quality gates enabled for this code.
See analysis details in CodeScene
Quality Gate Profile: Custom Configuration
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
krzywon
left a comment
krzywon
left a comment
There was a problem hiding this comment.
I think this is ready now. My only comment is outside the scope of this PR.
There was a problem hiding this comment.
The more I look at units.py, the more I think this needs to be added to .gitignore and just let the build system worry about it. Why do we have the same content twice?
3 participants
As mentioned in #172, users might want to track units that aren't necessarily SI measurements (e.g. Slices per Pizza, Pages per Book). This PR adds an
ArbitraryUnitsclass which can track these sorts of units while still interactive with the regularUnits andNamedUnits` classes.It also contains a modernisation of the
utest_units.pytest file so that the tests can be properly searched and controlled through pytest.This is a continuation of #188. A slip of the finger led to this branch being force pushed to
refactor_24, which caused GitHub to believe that the PR was merged. I fixed and rolled-back the branch, but it is a known github issue that the PR cannot be restored after.