move tests to tests folder #211

andrewgsavage · 2024-04-01T00:25:34Z

No description provided.

codecov · 2024-04-01T00:26:57Z

Codecov Report

Attention: Patch coverage is 93.54839% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 95.60%. Comparing base (e9b82f0) to head (925f74a).
Report is 2 commits behind head on master.

Files	Patch %	Lines
uncertainties/testing.py	88.88%	9 Missing ⚠️
uncertainties/unumpy/core.py	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
+ Coverage   95.53%   95.60%   +0.06%     
==========================================
  Files          17       13       -4     
  Lines        2085     1912     -173     
==========================================
- Hits         1992     1828     -164     
+ Misses         93       84       -9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

newville

Thanks!! This looks good to me.

andrewgsavage · 2024-04-01T09:58:55Z

Thanks!! This looks good to me.

This now shows the changes I had made for removing depreciations and 1to2, which I don't think you saw when you commented. I'll open a PR for 1to2 now that can be merged before this.

wshanks · 2024-04-01T15:31:18Z

Did you consider moving the tests out of the package (just a top level tests/ directory next to uncertainties/)? I think that is more standard.

I have occasionally found it useful to have tests in the main package so I can test the package with pip install package && python -m unittest package.tests and without separately cloning the repo, but I don't think pytest supports such execution of tests in an installed package. I think it always wants to run from a working directory containing the tests below it.

newville · 2024-04-01T15:47:04Z

+1 on moving tests out of the source tree and into a top-level tests folder.
But then the CI scripts need updating too.

wshanks · 2024-04-01T16:00:02Z

The CI scripts just do a plain pytest call, though it would be nice to specify tests/ so pytest doesn't waste time searching other files (not that there are many here). I mentioned adding a minimal pytest configuration (like this) in #205 (comment) because I think it might also help with that case (VS Code knowing to run pytest).

newville · 2024-04-01T17:07:21Z

@wshanks Ah, thanks, yes I see that now, and I completely agree.
I would probably go with the "add pytest config to pyproject.toml", but adding a pytest config file seems okay to me too.

andrewgsavage · 2024-04-01T18:06:10Z

interesting that it keeps the approval after I make new commits

I moved some functions that could be used by downstream libraries to uncertainties.testing

wshanks · 2024-04-01T21:56:06Z

interesting that it keeps the approval after I make new commits

I enabled dismissing approvals when new changes are pushed. I have found it to be annoying on other projects where the reviewer approves the spirit of the PR but then has to come back and approve a couple more times because of typos/linting. Here, it seems like we have a responsive enough group for it to be okay.

wshanks

This looks good. Besides my inline question, could we add this to pyproject.toml:

[tool.pytest.ini_options]
testpaths = ["tests"]

I don't know that we need to add any additional options at this time.

wshanks · 2024-04-01T22:10:26Z

tests/test_umath.py

-from . import test_uncertainties
-
+from uncertainties.testing import compare_derivatives, numbers_close
+from test_uncertainties import power_special_cases, power_all_cases, power_wrt_ref


It does not seem right to import from one test_ file to another. These functions do seem too specialized for uncertainties.testing though. Could we use one of the options in this StackOverflow question. Maybe the conftest.py and fixture option, or the pythonpath and helpers file option?

Adding testpaths = ["tests"] is not required for tests to work right? It's just a convenience that makes pytest discover tests more quickly? Is my understanding correct?

In the past I've put helper function needed for testing in an __init__.py file in the tests folder. These objects can then be imported within the test scripts. We could also have a tests/helper.py module to accomplish the same result.

Yea, importing across test files does feel a bit odd. The helpers.py solution is nice and simple.

Adding testpaths = ["tests"] is not required for tests to work right? It's just a convenience that makes pytest discover tests more quickly? Is my understanding correct?

Yes, that seems to be all it does functionally. My conjecture was that it might also help IDE's figure out that the project uses pytest as the test runner because otherwise there is no configuration indicating that pytest should be run, but I haven't tried to confirm that.

I like the move to helpers.py. We might want to add

testpaths = ["tests"] addopts = [ "--import-mode=importlib", ]

to pyproject.toml like the one StackOverflow answer suggests, but we could wait for that. Currently, pytest adds all the test directories to the Python path so you can do import helpers but it has been trying to move away from that to just importing the test files with importlib without modifying the Python path.

thanks for explaining, wouldn't have understood why otherwise

wshanks

I think this is good. I had one suggestion about adding more pytest configuration but it's not mandatory.

This commit removes: - the 1to2 code and documentation, - the compatability code that handled obsolete function calls, - the deprecations in these calls, - the tests that tested obsolete behaviour

wshanks · 2024-04-02T14:01:48Z

Also, should this fold in #210? Otherwise they will create merge conflicts with each other.

Removed unused imports in tests. Co-authored-by: andrewgsavage <andrewgsavage@gmail.com>

…into andrewgsavage-tests

andrewgsavage · 2024-04-02T20:09:58Z

I think this is good. I had one suggestion about adding more pytest configuration but it's not mandatory.

I tried doing this and it worked locally but I couldn't get it working with CI. I've reverted so this can be merged and we can do the importlib in another PR. Ready for merging now

The merge-base changed after approval.

wshanks · 2024-04-02T20:29:41Z

It looks good but it won't let me merge at the moment. The diff also doesn't look right in the GitHub web UI but looks right locally. I saw this in another repo recently as well.

The merge-base changed after approval.

jagerber48 · 2024-04-02T20:31:32Z

Can testing.py be moved into tests/helpers.py?

wshanks · 2024-04-02T20:32:52Z

Can testing.py be moved into tests/helpers.py?

It could be, but @andrewgsavage described the contents above as "I moved some functions that could be used by downstream libraries to uncertainties.testing"

andrewgsavage · 2024-04-02T20:34:52Z

hold up, looks like I'd deleted the contents of a file
e:fixed

wshanks · 2024-04-02T20:51:17Z

I see all the changes from #214 still in the web UI. Could you squash onto master and force push? Or does it show up correctly for others now?

jagerber48 · 2024-04-03T02:09:01Z

The testing.py concept seems strange to me.

I moved some functions that could be used by downstream libraries to uncertainties.testing

These functions are only used in the tests so I think it makes sense for them to be in the test helpers module. Is the idea that downstream users might want to use these functions? If so, then they should be part of public API and they need to be documented etc. And if they're meant to be part of the public API I think it is strange for them to be in a module named testing instead of a module name utilities or something.

newville · 2024-04-03T03:13:47Z

I am not ever going to be able to keep up with conversations at the pace here.

I do not fully understand the added uncertainties/testing.py file either. Do we want to support users using these tests?

I am working slowly on the docs, but this will take some time -- given my work schedule, into next week.

I deeply despise that codecov calls a PR unsuccessful if it loses a bit of coverage. Knowing the code coverage is fine. As a github check, codecov and its reports are warts. I will make a PR to copy the codecov config from lmfit.

andrewgsavage added 2 commits March 31, 2024 17:38

remove 1to2 and depreciations

aab68cf

move tests to tests folder

da08ae9

newville previously approved these changes Apr 1, 2024

View reviewed changes

newville force-pushed the master branch from 4f2693f to e9b82f0 Compare April 1, 2024 03:13

wshanks mentioned this pull request Apr 1, 2024

Added basic VSCode settings for pytest. #205

Closed

andrewgsavage added 2 commits April 1, 2024 18:35

move tests folder

45b6057

move things around to get tests running

9cb8085

remove refences to nose

88364a7

andrewgsavage dismissed newville’s stale review via 88364a7 April 1, 2024 21:49

wshanks reviewed Apr 1, 2024

View reviewed changes

andrewgsavage added 2 commits April 1, 2024 23:49

add tests to pyproject.toml

8f904f3

move power tests to helpers

97cbdf4

wshanks previously approved these changes Apr 2, 2024

View reviewed changes

remove 1to2 and deprecations (lmfit#214)

8421268

This commit removes: - the 1to2 code and documentation, - the compatability code that handled obsolete function calls, - the deprecations in these calls, - the tests that tested obsolete behaviour

importlib

f69d5fc

andrewgsavage dismissed wshanks’s stale review via f69d5fc April 2, 2024 19:34

willynilly and others added 5 commits April 2, 2024 20:36

Removed unused imported modules from tests. (lmfit#210)

57e2a20

Removed unused imports in tests. Co-authored-by: andrewgsavage <andrewgsavage@gmail.com>

Merge branch 'tests' of https://github.com/andrewgsavage/uncertainties …

d48d1ec

…into andrewgsavage-tests

importlib

a58a7e7

add tests.helpers to path

7a92f05

add tests.helpers to path

4b7ab86

andrewgsavage added 2 commits April 2, 2024 21:06

revert importlib

04070e7

revert importlib

a6fe054

wshanks previously approved these changes Apr 2, 2024

View reviewed changes

empty umat

925f74a

andrewgsavage closed this Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move tests to tests folder #211

move tests to tests folder #211

andrewgsavage commented Apr 1, 2024

codecov bot commented Apr 1, 2024 •

edited

Loading

newville left a comment

andrewgsavage commented Apr 1, 2024

wshanks commented Apr 1, 2024

newville commented Apr 1, 2024

wshanks commented Apr 1, 2024

newville commented Apr 1, 2024

andrewgsavage commented Apr 1, 2024

wshanks commented Apr 1, 2024

wshanks left a comment

wshanks Apr 1, 2024

jagerber48 Apr 1, 2024

andrewgsavage Apr 1, 2024

wshanks Apr 2, 2024

andrewgsavage Apr 2, 2024

wshanks left a comment

wshanks commented Apr 2, 2024

andrewgsavage commented Apr 2, 2024

wshanks commented Apr 2, 2024

jagerber48 commented Apr 2, 2024

wshanks commented Apr 2, 2024

andrewgsavage commented Apr 2, 2024 •

edited

Loading

wshanks commented Apr 2, 2024

jagerber48 commented Apr 3, 2024

newville commented Apr 3, 2024

move tests to tests folder #211

move tests to tests folder #211

Conversation

andrewgsavage commented Apr 1, 2024

codecov bot commented Apr 1, 2024 • edited Loading

Codecov Report

newville left a comment

Choose a reason for hiding this comment

andrewgsavage commented Apr 1, 2024

wshanks commented Apr 1, 2024

newville commented Apr 1, 2024

wshanks commented Apr 1, 2024

newville commented Apr 1, 2024

andrewgsavage commented Apr 1, 2024

wshanks commented Apr 1, 2024

wshanks left a comment

Choose a reason for hiding this comment

wshanks Apr 1, 2024

Choose a reason for hiding this comment

jagerber48 Apr 1, 2024

Choose a reason for hiding this comment

andrewgsavage Apr 1, 2024

Choose a reason for hiding this comment

wshanks Apr 2, 2024

Choose a reason for hiding this comment

andrewgsavage Apr 2, 2024

Choose a reason for hiding this comment

wshanks left a comment

Choose a reason for hiding this comment

wshanks commented Apr 2, 2024

andrewgsavage commented Apr 2, 2024

wshanks commented Apr 2, 2024

jagerber48 commented Apr 2, 2024

wshanks commented Apr 2, 2024

andrewgsavage commented Apr 2, 2024 • edited Loading

wshanks commented Apr 2, 2024

jagerber48 commented Apr 3, 2024

newville commented Apr 3, 2024

codecov bot commented Apr 1, 2024 •

edited

Loading

andrewgsavage commented Apr 2, 2024 •

edited

Loading