Merged
Conversation
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- sasdata/sasdata/dataloader/readers/danse_reader.py is usually changed with: sasdata/sasdata/dataloader/readers/tiff_reader.py
- sasdata/sasdata/dataloader/readers/red2d_reader.py is usually changed with: sasdata/sasdata/dataloader/readers/tiff_reader.py
Quality Gate Profile: The Bare Minimum
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
rprospero
approved these changes
Aug 1, 2025
rprospero
left a comment
rprospero
left a comment
There was a problem hiding this comment.
The code is clearly cleaner after the refactor. I've added a couple of comments, but I'm approving the PR.
| def test_coo_matrix_encode_decode(shape, n, m, dtype): | ||
|
|
||
| i_indices = | ||
| #i_indices = |
There was a problem hiding this comment.
Any reason not to delete this line entirely?
Contributor
Author
There was a problem hiding this comment.
I thought it could have been left as an aide-mémoire, but I'll take it out if there's no obvious reason.
sasdata/data_util/manipulations.py
Outdated
| data2d.data)] - dq_overlap | ||
| # def; dqx_data = dq_r dqy_data = dq_phi | ||
| # Convert dq 2D to 1D here | ||
| # Is the line below a bug? Should it be: dq_data = np.sqrt(dqx_data**2 + dqy_data**2) instead? |
There was a problem hiding this comment.
It looks like a bug to me. After all, if it is not a bug, then we never use the value of dqy_data
Contributor
Author
There was a problem hiding this comment.
My thoughts exactly. Do you think I should make the change?
Collaborator
There was a problem hiding this comment.
Definitely a bug. Is this also in the master branch? If so, we should fix it there as well as here.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- sasdata/sasdata/dataloader/readers/danse_reader.py is usually changed with: sasdata/sasdata/dataloader/readers/tiff_reader.py
- sasdata/sasdata/dataloader/readers/red2d_reader.py is usually changed with: sasdata/sasdata/dataloader/readers/tiff_reader.py
Quality Gate Profile: The Bare Minimum
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
This was referenced Aug 4, 2025
DrPaulSharp
added a commit
that referenced
this pull request
Aug 11, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
DrPaulSharp
added a commit
that referenced
this pull request
Aug 11, 2025
* Test the interpolation on the trend works. It doesn't :( * Fixed call to interpolate function. * Sasdata contains a list of quantities. Not named quantities. * Make these properties. * Missing self perameters. * Store the dataset type inside SasData. * Try using a dict with validation. * Make this subscriptable. * Changes in accordance with quantities changes. Some of them probably don't work but I'll fix them later. * Wrote some docstrings. * Add a docstring for the params class. * Fixed line endings. I think I resolved a merge conflict incorrectly. * Remove todo comment. * Added docstrings. * Remove this. * Use full function name here. * Return a dictionary of quantities; not a list. * Merge uncertainties returns a dict. * Add a dataset type to the ascii reader params. * Updated the SasData construction call. * Add with standard error for quantities. * Fixed type hinting. * Return a quantity not a named quantity. * Leave the original Q axis in. * Remove python interpreter header. * Remove this comment as I don't think its true. * Skeleton implementation for ordinate and abscissae * These are the wrong way round. * Match statements are causing problems; use if. * Implement abscissae for 2D data. * Wrote a test for 1d data. * Use the value property. * 2D test. * Bring guess into sasdata. * Bring default logic from SasView here. * Make the default value a param. * Function for making the guesses. Useful for testing. * Forgot return statement. * Column unit can be none. * If there are extra columns than expected, ignore. * Wrote a test for the ASCII reader. * Need to get the full filename. * Don't use filename to get filename :P * Property that doesn't include ignore columns. * It seems I'm a bit sleepy today. * Use the new property. * Find can accept more locations in the future. * Missing import. * Second test. * Uncertainties should have the same units. * Add slow cubic interpolation * Fix first for loop * Remove second for loop from conversion_matrix calculation * Parameterize tests over interpolation orders * Create test option for displaying diagnostic plots * More interpolation test into main test directory * Temporarily add a magnetic category. * Fixed typo. * Don't compare against the quantity. * Need to convert to list before array. * Apply all twice. * Use brackets properly. * Add tests for dataset * Handle collimations directly * Start properly descending node tree * Make aperture part of collimation * Move to dataclasses * Parse source * Parse source in instrument * Start fixing Detector setup * Get Detector name and wavelength properly * Better unit parsing * All the detector parts * Fix up aperture * Instrument is a data class * Keyword only dataclasses * Make the Instrument metadata optional * Parse sample data * Refactor our conditional parsing code * Process Metadata as a dataclass * Simplify radiation handling * Include high leve metadata in metadata * Metadata is a dataclass * Remove unused values * Properly load and test multiple entries * Minor cleanup * Flag node types for parses * Fixup after rebase * Clean up import lint * More lint * More handling of the change to dict * All metadata needs to be optional to accomodate other file formats * Fix up types * Collimation is optional * Fix imports * Replace optional with proper sum type * Remove unused imports * Fix parsing of unit full names * Remove testing bit * Don't edit the file with a giant "Don't edit this file" header * Handle null title and sample in summary * Start parsing XML * Create _load_text helper * Parse process * Refactor with helper functions * Fully parse xml metadata * Parse IData in XML * Remove extraneous line in hdf reader * Trial multiple files * Fix aperture load * Support multiple cansas versions * More IData column types * Start adding official tests for xml file loading * Add singular alias to degrees unit * Better work at handling comments * Refactor text parsing to better handle comments * More tests of xml * Mark weird units, but continue parsing * Add last xml test * Fix parsing of BeamSize * Add micron unit alias * Handle singular unit names in parsing * Always print summary in same order * Reinstate summary output when xml loader is used as a main module * Factor our finding cansas version into its own file * More docstrings * Fix typo in comment * Fix type hints * Provide default name for unnamed data sets * Better default names for SasData entries * Process can have multiple terms, which may be quantites * Fix hdf process term parsing * Correctly parse process notes * Add type hint to load_data filename * More concise source parsing * Whitespace fix * Typehint on load_data * Fix up Source parser * Add raw data to xml metadata * Add raw handling to reader * Enable raw data filter * Start testing data filter * Run through code formatter * Simplify metadata filtering * Raise KeyError instead of ValueError * Fix up ascii reader tests * Update creation of SasData in trend There is no longer a _raw_metadata parameter * Properly compare named units with unnamed units * Enforce loading test reference files in UTF-8 * Skeleton framework for SESANS data * Start parsing sesans header * Start parsing SESANS sample metadata * Include SESANS angle metadata * Multiple SESANS files in reader test * Parse actual data from SES files * Add Raw SESANS Node Data * Update XML test references to include apertures * SESANS metadata as a process, not an aperture * Fixup lint * Make changes suggested in PR review * Fix simple typos from rebase * Add equality testing for quantities * More tests for quantities * Fix meshmerge calculation * Ruff format. * This comment was repetitive. * Ruff format. * Function to guess the dataset type. * Function for loading a file with default params. * Ruff format. * Remove imports ruff is complaining aren't used. * Added a test to make sure 2d data gets read right. * Makes sure the dataset type gets guessed. * Use basename for sasdata ascii name. * Be consistent with how basename is called. * Move comment to avoid odd Ruff format. * Move comment again. * Fixed test. * Created an import metadata function. To replace the old metadata loading. * Remove the old function. * Use the new import function. * Remove these imports. Not need anymore. * Need to use keywords for this dataclass. * Import the mumag data for testing. * Added a case for mumag data. * Added test for loading ascii data with metadata. * Use params filenames not just filenames. * Fixed column parameters. * Doh. Missing commas on filename list. * Roll the raw metadata into the metadata object. * Need to fill in all the parameters. * Forgot process :P * Consider both of these. * Combine both metadata so we go through all of them * Raw metadata is in lists. * I don't know what these decimals were. Truncate them for now. * Start implementing ModellingRequirements * Start testing modelling requirements * Start adding better tests for ModellingRequirements * Flip order of parameters on compose * Don't assume that Sesans includes smear * Enable left composition by null model * support right addition of NullModel * Allow preprocess and postprocess steps * Start performing Hankel transform for SESANS * Ignore slit smearing before SESANS * Pull sesans metadata from file * Add unit test for Hankel transform * Fix units with error calculation * χ² squared based test * Code Review Suggestions * Fix up rename in compose * Fix uncertainty in SESANS parser The error in a Quantity should *not* be another quantity, but a value in the units of its parent. * Update SESANS units in unit_kinds * Applies auto fixes for ruff rule F401 * Applies auto fixes for ruff rule E714 * Applies auto fixes for ruff rule F541 * Bumps minimum python version to 3.12 * Test should fail when there's no data. This is a bit of a bodge for now, but a more permanent fix will be applied in the unit test refactor. * Added a get default unit function. Also Ruff format. * Use the new get default unit function. * Pass in the unit group as well. * Return value if it isn't None. * Set the dataset type properly. * Expect 2D test to fail. * Use Quantity in models * Don't use a quantity for model post processing * Basic Pinhole model * Refactor pinhole tests * Fix radius bug in smearing * More realisting δq * Combine Smearing and Sesans tests * Define model metadata in constructor * Better testing range for pinhole * Add slit smearing * Remove old commented code * Vectorise slit calculation This runs about 80% faster * Check combination of slit models * Decrease logical depth of linear_extrapolation * Decrease logical depth of slit_resolution * Decrease logical depth of geometric_extrapolation * Apply PR changes * Fixes ruff linting errors (#140) * Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments * Adds Pyupgrade (UP) ruleset and rules SIM118 & SIM300 to ruff linter (#144) * Applies fixes for the pyupgrade (UP) ruff ruleset * Adds fixes for ruff rules SIM118 and SIM300 * Addresses review comments * Applies fixes for isort (I) ruff ruleset (#146) * Fixes whitespace errors * Fix hours, days, and years * Add command for explicitly formatting quantity units * Fixes linting error --------- Co-authored-by: James Crake-Merani <James.Crake-Merani@stfc.ac.uk> Co-authored-by: James Crake-Merani <james@jamescm.co.uk> Co-authored-by: Adam Washington <adam.washington@stfc.ac.uk>
DrPaulSharp
added a commit
that referenced
this pull request
Aug 11, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
DrPaulSharp
added a commit
that referenced
this pull request
Oct 15, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
DrPaulSharp
added a commit
that referenced
this pull request
Nov 5, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
DrPaulSharp
added a commit
that referenced
this pull request
Nov 7, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
DrPaulSharp
added a commit
that referenced
this pull request
Dec 8, 2025
* Fixes ruff linting errors * Adds ruff to pyproject.toml * Further fixes for refactor branch * Addresses review comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes around half of the remaining ruff linting violations for the E and F rulesets. There are 45 ruff errors left over.