Conversation
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.
![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.
rprospero
force-pushed
the
172_unrecognized_units
branch
from
662433f to
6e85365
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.
| # | ||
| # | ||
|
|
||
| class ArbitraryUnit(NamedUnit): |
There was a problem hiding this comment.
Arbitrary unit (a.u.) is used for SAS data not on absolute scale. I think UnknownUnit or similar would better match the current naming scheme and not cause confusion.
There was a problem hiding this comment.
You're right about the confusion. I've switched to UnknownUnit
| numerator: str | list[str] | dict[str, int], | ||
| denominator: None | list[str] | dict[str, int]= None): |
There was a problem hiding this comment.
What should these strings and/or lists look like? Documentation on this would be helpful. Currently, the unit converter allows a*a, a**2, and a^2. What happens if a^2/b is passed as the numerator? Does it matter?
There was a problem hiding this comment.
Likely it won't matter, since UnknownUnit should only be called after a failed parse that has already split on those characters. However, just in case, I've added validation that throws a runtime error if invalid characters are in any part of the unit.
There was a problem hiding this comment.
This is contained in the _valid_name static method
| case str(): | ||
| self._numerator = {numerator: 1} | ||
| case list(): | ||
| self._numerator = {} | ||
| for key in numerator: | ||
| if key in self._numerator: | ||
| self._numerator[key] += 1 | ||
| else: | ||
| self._numerator[key] = 1 | ||
| case dict(): | ||
| self._numerator = numerator | ||
| case _: | ||
| raise TypeError |
There was a problem hiding this comment.
This match/case block is almost exactly the same as the one for denominator. Could this be made into a separate function that returns the resulting dictionary?
There was a problem hiding this comment.
Refactoring out the repetition also made the string validation simpler. The separate function is now the _parse_arg class method.
| case (_, []): | ||
| return " ".join(num) | ||
| case ([], _): | ||
| return "1 / " + " ".join(den) |
There was a problem hiding this comment.
This will give something like 1 / A B C which will create ambiguity for B and C. Maybe return "1 / (" + " ".join(den) + ")"? I would suggest something similar in the default case as well.
There was a problem hiding this comment.
I've added parentheses, but only in the case where there are multiple terms in the denominator. Thus, it will appear as "1 / (A B C)", but the single term will still appear as "1 / A"
| return str(self) | ||
|
|
||
| @staticmethod | ||
| def parse(unit_string: str) -> "Unit": |
There was a problem hiding this comment.
standardize_units and _format_unit_structure in the existing sasdata.data_util.nxsunit module already has a basic version of this you might want to look at.
|
This has NOT been merged, despite what GitHub claims. Rather, the discussion continues in #190. |
2 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.