Refactor 24 trend#198
Conversation
There is no longer a _raw_metadata parameter
* Data sketch * Units now available and grouped * More units, towards formatting * Units and accessors draft ready to begin tests on * Some tests * Work towards new data object * Accessor changes * Remove duplicate evaluate function. * Removes duplicate code * Fills out log operations * Adds log evaluation tests * Tidies up operations code * Corrects derivative for Div operation * Adds tests * Refactors Log operation * Ruff format * Removes unnecessary file --------- Co-authored-by: lucas-wilkins <lucas.wilkins@stfc.ac.uk> Co-authored-by: James Crake-Merani <James.Crake-Merani@stfc.ac.uk>
* Adds exp operation and corrects log derivatives * Adjusts math library imports * Adds trig operations * Adds evaluation tests * Corrects unary clean signatures * Adds tests * Adds clean for exp and ln * Adds parameterised fixtures * Addresses review comments
…h is defined by one or the other, and they are not the same. IMO, we assume the width values given in the dQ column correspond to sigma.
…et methods do not exist. Valid solution or should we implement _raw_metadata and children?
…Using a dict now for the axis and changed also name to plural: Trend.trend_axes, so unit tests changed accordingly
![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.
DrPaulSharp
force-pushed
the
refactor_24
branch
3 times, most recently
from
90413d7 to
dd691e0
Compare
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.
There was a problem hiding this comment.
No quality gates enabled for this code.
See analysis details in CodeScene
Absence of Expected Change Pattern
- sasdata/sasdata/dataloader/readers/red2d_reader.py is usually changed with: sasdata/sasdata/dataloader/readers/ascii_reader.py, sasdata/sasdata/dataloader/readers/tiff_reader.py
Quality Gate Profile: Custom Configuration
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
With the merge conflicts due to the rebase, I'm struggling to review this. I would suggest you merge the refactor branch into this one and fix and merge conflicts. Until then, I'm not sure what changes you made versus what came from the rebase. |
|
Yes, I fully understand. I have created a new branch starting directly from the last refactor_24. The diffs for 2 files there (metadata.py and trend.py) are lost, but I think it is much clearer there and in any case I don't think that I would be able to recover this any better. Give a look to the new branch and then probably we can delete this one. |
|
Closing in favor of #216 |
8 participants
Proposition to modify the original Trend class. The idea is that one can define as many axes as desired, using a dict to store them. Each axis can be linked to a metadata (given a list with the path to the metadata as before) or generated "on the fly" (in this case one has also to give a list, with the constraints that the first element must NOT be a string and the number of elements in the list must be equal to the number of SasData objects in data. Are these limitations OK)?
If it is agreed that this is the right way, next steps should be to clean up a little bit the code and add new utests.