Skip to content

change: Handle uncertainties as None rather than 1s - #193

Merged
sbillinge merged 11 commits into
diffpy:v3.3.0from
cadenmyers13:fix-uncertainty-none
Aug 5, 2026
Merged

change: Handle uncertainties as None rather than 1s#193
sbillinge merged 11 commits into
diffpy:v3.3.0from
cadenmyers13:fix-uncertainty-none

Conversation

@cadenmyers13

Copy link
Copy Markdown
Contributor

Originally, dyobs was set to 1 if not present. I assume this is the case because residual = (ycalc - yobs) / dyobs would fail because you divide by None. This handles that by ensuring the interal calculation of residuals preserves the dyobs=1 behavior while the user will see dyobs = None so that there is no confusion about uncertainties being present.

@cadenmyers13 cadenmyers13 changed the title change: Handle uncertainties in y as None rather than 1s change: Handle uncertainties as None rather than 1s Aug 4, 2026
Comment thread tests/test_fitrecipe.py
(lambda xobs: linspace(0.1, 1.0, len(xobs)), True),
],
)
def test_refine_with_and_without_uncertainty(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests that the refinement still works with or without uncertainties

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.54%. Comparing base (0b16bfd) to head (29ce00d).
⚠️ Report is 4 commits behind head on v3.3.0.

Additional details and impacted files
@@            Coverage Diff             @@
##           v3.3.0     #193      +/-   ##
==========================================
+ Coverage   79.08%   81.54%   +2.45%     
==========================================
  Files          25       25              
  Lines        3821     3852      +31     
==========================================
+ Hits         3022     3141     +119     
+ Misses        799      711      -88     
Files with missing lines Coverage Δ
tests/conftest.py 90.90% <100.00%> (-2.00%) ⬇️
tests/test_fitrecipe.py 99.85% <100.00%> (+<0.01%) ⬆️
tests/test_pdf.py 56.36% <100.00%> (+4.18%) ⬆️
tests/test_profile.py 99.54% <100.00%> (+0.02%) ⬆️
tests/test_profileparser.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cadenmyers13

Copy link
Copy Markdown
Contributor Author

@sbillinge ready for review

@sbillinge sbillinge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see comments. Final decision is to use as_list() in all the tests and set the dyobs attribute to None and not any kind of array.

Comment thread tests/test_fitrecipe.py
[
# C1: No uncertainties are observed, as for a file with no
# uncertainty column.
# Expected: The refinement converges and dyobs stays None.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment; "return a function that evaluates dyobs on the array of xobs for insertion into the build_recipe_with_uncertainty fixture."

Comment thread tests/test_fitrecipe.py
[
# C1: No uncertainties are observed.
# Expected: dy falls back to one everywhere, so the residual is
# unweighted.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste the comment from above here in case in the future we are looking just at this test case.

Comment thread tests/test_profile.py Outdated
prof = Profile()
# The parsed x, y and dy arrays are copied onto the observed profile.
# Uncertainties on x are dropped, since srfit treats the independent
# variable as having no uncertainty, although for PDFs uncertainties in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this last statement about uncertainties in PDF

Comment thread tests/test_profile.py
[2.0, 2.1, 2.2],
[0.2, 0.4, 0.6],
),
# C2: File has two columns, so no uncertainties are present.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add three column data

Comment thread tests/test_profile.py Outdated
actual_xobs = prof.xobs.tolist()
actual_yobs = prof.yobs.tolist()
# Unavailable uncertainties are None rather than an array.
actual_dyobs = None if prof.dyobs is None else prof.dyobs.tolist()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use numpy.all_close or sthg like that?

Comment thread tests/test_profile.py Outdated
actual_xobs = prof.xobs.tolist()
actual_yobs = prof.yobs.tolist()
# Unavailable uncertainties are None rather than an array.
actual_dyobs = None if prof.dyobs is None else prof.dyobs.tolist()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define

import numpy as np


def allclose_with_none(a, b, rtol=1e-05, atol=1e-08):
    a = np.asarray(a)
    b = np.asarray(b)

    if a.shape != b.shape:
        return False

    # Check if None positions match exactly in both arrays
    none_a = a == None
    none_b = b == None
    if not np.array_equal(none_a, none_b):
        return False

    # Compare non-None elements as float arrays
    mask = ~none_a
    return np.allclose(
        a[mask].astype(float), b[mask].astype(float), rtol=rtol, atol=atol
    )

then use allclose_with_none

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the helper function in conftest.py

@cadenmyers13

Copy link
Copy Markdown
Contributor Author

@sbillinge ready for review

@cadenmyers13

Copy link
Copy Markdown
Contributor Author

@sbillinge Can you check this out when you get a chance? The sooner we can get this released the better

@sbillinge
sbillinge merged commit 02ca2b4 into diffpy:v3.3.0 Aug 5, 2026
6 checks passed
@cadenmyers13
cadenmyers13 deleted the fix-uncertainty-none branch August 5, 2026 03:39
sbillinge added a commit that referenced this pull request Aug 8, 2026
* deprecate addProfileGenerator

* update api docs

* mock utils and pyobjcryst for doc rendering

* news

* restore api docs

* revert conf.py commit

* deprecate addContribution and replace with add_contribution

* news

* deprecate setEquation and replace with set_equation

* more set_equation replacements

* news

* reorder dep message to prevent merge conflicts

* one final setEquation switch in tests

* pushFitHook deprecation

* popFitHook deprecation

* popFitHook dep message

* getFitHooks and clearFitHooks deprecation

* setWeight deprecation

* news

* news pt2

* addParameterSet deprecation

* news

* docstring typo

* deprecate getEquation in fitcontribution.py

* deprecate getEquation from builder.BaseBuilder

* news

* news typo

* deprecate SimpleRecipe.loadParsedData

* deprecate Profile.loadParsedData

* deprecate Profile.setObservedProfile and SimpleRecipe.setObservedProfile

* news

* setCalculationRange

* setCalculationPoints

* change rebinArray to rebin_array

* make _rebin_array private

* missed deprecation tag in SimpleRecipe

* setResidualEquation deprecation

* getResidualEquation deprecation

* news

* news again

* add test for remove and add param_set

* removeParameterSet deprecation

* scalarResidual deprecation

* addVar deprecation

* delVar deprecation

* newVar deprecation

* news

* news 2

* isFree deprecation

* getValues deprecation

* fix typo in base name

* getNames deprecation

* getBounds deprecation

* getBounds2 deprecation

* add test for boundsToRestraints

* boundsToRestraints deprecation

* make bounds to restraints docstring better

* news

* news 2

* make new test cleaner

* remove minus sign typo causing tests to fail

* formatResults deprecation

* printResults deprecation

* add tests for printResults and formatResults

* write test for saveResults

* test for the addition of a header to results

* saveResults deprecation

* news

* set fixtures to scope=function

* add constraints and restraints for more strict testing

* add initialization test and method

* news

* remove accidental commit

* add test for ValueError

* AUTHORS.rst

* CODE-OF-CONDUCT.rst

* cookiecutter.json

* LICENSE.rst

* LICENSE.rst pt2

* MANIFEST.in

* pyproject.toml

* README.rst

* release_checklist.md

* build-and-publish-docs-on-dispatch.yml

* build-wheel-release-upload.yml

* matrix-and-codecov.yml

* README.rst pt2

* conf.py

* index.rst

* license.rst

* diffpy.srfit.rst

* __init__.py

* __init__.py

* __init__.py

* srfit_app.py

* version.py

* news

* [pre-commit.ci] auto fixes from pre-commit hooks

* resultsDictionary deprecation, added to FitResults

* news

* test for resultsDictionary

* improve test

* add comments describing tests

* rm merge-to-main duplicate workflow

* add __init__ header to README

* pull PR to list branch to fix docformatter failure

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix conftest fixture so it creates new recipe object on each call

* update fixture initial values to be more different

* update expected to accound for the slight difference now

* feat: initialize FitRecipe from a results file or object

* test: tests for the new feature

* add utils objects to get dict from results object or file

* news

* fix accident in test_fitresults

* use get_results_dictionary method to get params_dict

* initializeRecipe deprecation

* add better docstrings to fitrecipe and fitresults

* add 'The' to docstrings

* do linspace with odd number of points

* revert recipe fixture back, explicitly set scope to 'function', and add a new helper function that builds recipe for init testing

* add assert recipes arent equal

* add fixture that returns two identical recipes

* add second recipe to fixture build_recipe_one_contribution

* change fixture name to be plural (build_recipes_one_contribution)

* rm deprecated six and other deprecated python2 objects

* setValue deprecation

* news

* skip test_speed.py

* ignore test_speed.py in codecov

* iterPars deprecation

* registerCalculator deprecation

* registerFunction deprecation in EquationFactory and RecipeOrganizer

* registerStringFunction deprecation

* evaluateEquation deprecation

* isConstrained deprecation

* replace par with parameter

* getConstrainedPars deprecation, and write a test for it :)

* small fix

* clearConstraints deprecation, and write small test

* clean up and fix variable names in restrain and constrain methods

* minor docstring typos

* news

* constrain and unconstrain deprecation

* change name to register_restraint

* restrain deprecation

* unrestrain deprecation

* clearRestraints deprecation

* equationFromString deprecation

* news

* constrain_parameter --> add_constraint

* unconstrain_parameter --> remove_constraint

* add_restraint --> add_penalty

* remove_restraint --> remove_penalty

* register_restraint --> register_penalty

* clear_all_restraints --> clear_all_penalties

* change lb to lower_bound everywhere

* change ub to upper_bound everywhere

* add_penalty --> add_soft_bounds

* remove_penalty --> remove_soft_bounds

* register_penalty --> register_soft_bounds

* clear_all_penalties --> clear_all_soft_bounds

* add more to add_soft_bounds docstring

* more to docstring

* processMetaData --> _process_metadata

* isConst --> is_constant

* change Attributes --> Parameters in all docstrings

* boundRange --> bound_range

* boundWindow --> bound_window

* write tests for bound_window and bound_range

* news

* minor docstring fix

* deprecate PDFParser

* build parse_file and deprecate getNumBank, selectBank, getFormat, getData, and getMetaData

* add ProfileParser to __init__ so it can be imported from fitbase

* add test files as conftest fixture

* use ProfileParser in PDFContribution, replacing PDFParser

* Use ProfileParser instead of PDFParser for fitrecipe testing

* getData --> get_data, and getMetaData --> get_metadata in Profile

* add set_parsed_profile test with ProfileParser

* update examples with new parser

* update test_pdf to reflect new parser

* update old formatted test data to get parser to pass tests

* add test for ProfileParser

* update sas module and test to reflect new changes

* news

* rm comment

* blank commit to fix pre-commit

* run pre-commit autoupdate and run pre-commit

* set python version to 3.13 to pin pre-commit CI

* pin only docformatter to python3.13

* rm python3.13 pin on pre-commit-config for now

* pip pre-commit docformatter to 3.13

* fix: fix iterpars behavior

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: rename par to parameter

* chore: change docstring, refind test description and structure

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: change to if else statement in the logic

* chore: refactor pytest fixtures and sync `pyproject.toml` and pre-commit file with main (#188)

* sync pre-commit file and pyproject.toml with main

* refactor pytest fixtures to remove use of triple quotes

* chore: news

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix docformatter and black conflicts

* chore: news v2

* clean up ugly docstring formating

* empty commit

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* chore: merge remaining `main` edits into `v3.3.0` (#189)

* fix: fix Qmax update after load new parsed data

* chore: add news item

* chore: rename the private method

* build: add diffpy.structure back to requirement

* chore: rebuild the documentation with proper metadata handling

* chore: update pre-commit (#184)

* chore: update pre-commit

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: fix docformatter and black conflict

* chore: run pre-commit for file I missed

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: add #FIXME workaround

* chore: add #FIXME workaround

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: add another #FIXME workaround

* chore: add another #FIXME workaround

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: moved import statement to the top of import modules

* [pre-commit.ci] auto fixes from pre-commit hooks

* chore: added try except workaround

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* release: add support for python 3.14 and remove for 3.11 (#187)

* chore: news

* chore: remove duplicate FIXME line

* chore: fix improperly merged method in ProfileGenerator

---------

Co-authored-by: stevenhua0320 <r.hua@mail.utoronto.ca>
Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>
Co-authored-by: Daniel Sirakov <danielsirakov73@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* skpkg: update project to latest scikit-package template (#190)

* skpkg: update project to latest scikit-package template

* Reorganize update message in changelog

Moved the update of the project template from 'Changed' to 'Fixed' section.

---------

Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>

* change: Handle uncertainties as `None` rather than `1`s (#193)

* fix: return None instead of 0 for unavailable dx/dy uncertainties

* test: update and add tests for None uncertainty behavior

* chore: add news entry for uncertainty None fix

* test: add tests to ensure new handling of uncertainties doesn't break refinements

* change: update profile.set_observed_profile() to not convert dy to ones if dy is None

* chore: update news file

* Update tests to make it easier to not use a testdata file for interpretability

* add as_list as a fixture

* add comment about how the fixture is used in test

* add three_column case

* use as_list as a fixture

* Fix: Restore extensibility of `ProfileParser` and use `load_data` from `diffpy.utils` (#196)

* Update examples to use the correct parser and add description of how to create your own parser in docs

* update old metadata format to new xPDFsuite and pdfgetx headers. One testdata file for each

* add test for new parser behavior

* change how ProfileParsers work by allowing user to create their own parsers for metadata and the data itself

* update the pre-existing news file with these changes

* Make PDFParser identical to ProfileParser now that ProfileParser defaults to load_data

* update old data metadata formats in testdata files to mirror xPDFsuite (neutron data) and pdfgetx (xray data)

* updated expected metadata in test

* fix docs description of ProfileParser

* update news

* minor docstring fix

* tests for adding metadata upon parsing a file with ProfileParser

* feat: allow user to append metadata when parsing a file with profileparser

* doc: Update docstrings across `fitbase/` and `pdf/` to group standards (#198)

* Update docstrings in fitbase to numpy/group standards

* update docstrings in pdf to np/group standards

* update docstrings in interface and structure to group standards

* news

* use recipe.plot_recipe() to plot fits in examples

* remove plot_results in favor of recipe.plot_recipe() method

* Change plot_recipe to display the contribution name as the title

* add tests for the new plot_recipe behavior

* rm accidentally commited output file

* tidy a test up

* empty commit for CI

* dep: Deprecate characteristic functions containing camel case (#197)

* Deprecate characteristic function camel case names

* Change cf names in example scripts

* add tests for characteristic functions

* news

* deprecate constrainAsSpaceGroup

* update news

* change: emit a warning if a characteristic function goes to a negative value during refinement. The output goes to zero

* Add tests for new CF behavior for non-physical input parameters

* cf news

* empty commit for CI

* change: update how the deprecated characteristic functions handle the signature changes (#199)

* revert parameters in old function signatures to prevent errors when the name is specified in the signature

* Change how the deprecated characteristic functions handle the signature change

* Add test cases for the deprecated functions signature changes

* news

---------

Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: stevenhua0320 <r.hua@mail.utoronto.ca>
Co-authored-by: Daniel Sirakov <danielsirakov73@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants