fix: ruff pt012 violations #16158

paverett · 2024-03-05T01:26:41Z

Description

This pull request is to resolve all of the PT012 ruff violations in the astropy repository.

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2024-03-05T01:27:15Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

nstarman

Thanks @paverett for the PR! Some quick comments on cosmology and units.

nstarman · 2024-03-05T02:05:25Z

astropy/cosmology/flrw/tests/test_base.py

@@ -1019,8 +1019,6 @@ class HASOde0SubClass(cosmo_cls):
                def __init__(self, Ode0):
                    pass

-            _COSMOLOGY_CLASSES.pop(HASOde0SubClass.__qualname__, None)


These lines should not be removed.

From my testing of putting a print statement before and after the subclass init. I don't believe this line is getting called. It will error on the subclass init and never get to this line. And I can't move it out of the context manager or else we get an UnboundLocalError. I'm not sure the best way to keep this line in.

That's an interesting issue. It's important to make sure that _COSMOLOGY_CLASSES does not contain HASOde0SubClass after the test. Perhaps for now you can put the line in a try: ... except: UnboundLocalError: ...?

If you want to ensure _COSMOLOGY_CLASSES gets restored after the test then the best way to achieve that is with a yield fixture.

I'm not sure I see how to use a yield fixture here. From reading the documentation it seems like it is used in place of a return. But in this test I dont see a use for a return even from a helper function. The TypeError is raised as soon as the class is declared. We can't return the class definition.

In fact I don't even think the try block will doing anything. The class is declared in the with statement. We can't reference it anymore so the try will always go to the except block. Not sure the best way to proceed on this one.

So if I'm understanding it right. Everything before the yield happens before the test then when the test completes everything after the yield runs. So I could deep copy _COSMOLOGY_CLASSES before, then after the test runs set _COSMOLOGY_CLASSES back to the deep copy. Like nothing ever happened. However when I tried this broke other tests, raising Keyerrors in other tests like test_w0cdm.py and test_parameters.py and some others. All keyerrors happen while trying to cleanup by popping a test subclass from _COSMOLOGY_CLASSES. My approach must not be correct or I must be missing something.

You should post the exact code you tried.

In astropy/cosmology/flrw/tests/conftest.py

import pytest import copy from astropy.cosmology import core #imports are where the rest of the imports are, just showing what I imported. @pytest.fixture def clean_cosmology_classes(): original = copy.deepcopy(core._COSMOLOGY_CLASSES) yield core._COSMOLOGY_CLASSES = original

In astropy/cosmology/flrw/tests/test_base.py

@pytest.mark.usefixtures("clean_cosmology_classes") def test_init_subclass(self, cosmo_cls): """Test initializing subclass, mostly that can't have Ode0 in init.""" super().test_init_subclass(cosmo_cls) with pytest.raises(TypeError, match="subclasses of"): class HASOde0SubClass(cosmo_cls): def __init__(self, Ode0): pass

Test ran pytest astropy/cosmology

It is best to replace the teardown part of your fixture (i.e. core._COSMOLOGY_CLASSES = original) with

core._COSMOLOGY_CLASSES.clear() core._COSMOLOGY_CLASSES.update(original)

The latter avoids assignments, so it is guaranteed to only modify the items in _COSMOLOGY_CLASSES and not its identity.

Gotcha makes sense, thanks for the help!

nstarman · 2024-03-05T04:23:39Z

astropy/units/tests/test_format.py

    with pytest.raises(ValueError), warnings.catch_warnings():
-        # ct, dex also raise warnings - irrelevant here.
-        warnings.simplefilter("ignore")


This shouldn't have been moved. If ruff did this, then this should be reported upstream.

PT012 is not among rules Ruff can fix automatically.

I moved it. Not sure the best way to satisfy PT012 and keep it ignoring warnings. Right now I can think of three options.

Add pytest.mark.filterwarning("ignore") to the function definition.

Split the context managers out and have the with pytest.raises be the inside one.

Add a helper function which contains the lines below the context manager.

Any other options?

I like 1 and 4 (add a noqa).

Why can't you just have nested with-blocks?

Yes it complains about the warnings.simplefilter("ignore"). Well more that there are two lines under the raises statement.

Nested with blocks like this?

with pytest.raises(ValueError): with warnings.catch_warnings(): # ct, dex also raise warnings - irrelevant here. warnings.simplefilter("ignore") u_format.VOUnit().parse(string)

If so I tried that and ruff didn't like it.

Doesn't this work if you change the order of the with-statements?

Actually this is not testing for an exception from the warnings calls (don't see how they could raise a ValueError, unless perhaps one passes an invalid string to simplefilter()), so I think they might be moved outside the pytest.raises altogether.

As long as it is inside catch_warnings - otherwise the filter does not get reset.

Yes, I think this way around

with warnings.catch_warnings(): # ct, dex also raise warnings - irrelevant here. warnings.simplefilter("ignore") with pytest.raises(ValueError): u_format.VOUnit().parse(string)

it should work correctly and hopefully be accepted by Ruff.

eerovaher

It is very good to enforce PT012 because apparently we had tests that seem to have been written assuming that if there are multiple lines in a with-block that can raise an error then pytest.raises() checks all of them, when in fact it only checks the first error and the following lines in the block never get executed at all.

I do have some comments about the changes to the code, even though I did not do a thorough review.

eerovaher · 2024-03-05T15:50:25Z

astropy/convolution/tests/test_kernel_class.py

        with pytest.raises(TypeError, match=r".* allowed .*"):
            AiryDisk2DKernel(2, array=x)
+        with pytest.raises(TypeError, match=r".* allowed .*"):
            Box1DKernel(2, array=x)
+        with pytest.raises(TypeError, match=r".* allowed .*"):
            Box2DKernel(2, array=x)


This should clearly be a parametrized test

Makes sense. Should I move the Ring2DKernel to its own test? It takes 3 parameters when the rest only take 2. Not sure the best way to handle that in a parametrized test.

@WilliamJamieson, do you know why Ring2DKernel needs to have a different number of arguments than everything else?

It has an inner and an outer radius, so does need two required parameters – could parametrise over a tuple size and then call testkernel(*size, array=x).

eerovaher · 2024-03-05T15:53:10Z

astropy/io/fits/scripts/fitscheck.py

Why would addressing PT012 require editing this file?

I moved

astropy/astropy/io/fits/tests/test_fitscheck.py

Line 24 in 8bcf4a6

out = capsys.readouterr()[0]

out of the with statement in the test to satisfy PT012. When I ran pytest it uncovered a bug, the variable out was actually equal to pytest <version> and not fitscheck <version>. I added prog to the argument parser of this file so it would return with the program name instead of pytest. Same with the other three scripts.

It's not obvious to me that boilerplate code is the best solution to a minor problem that arises from the tests calling the main function directly instead of executing the code here as a stand-alone script. We could instead modify the tests so that they call the scripts as separate processes, which would be much more similar to their intended usage.

In any case if addressing PT012 reveals actual bugs then they should be fixed in separate pull requests so that the fixes could be backported.

Something like import subprocess then a subprocess.popen("fitscheck --version")? Then check from there? Not sure if there is a module in astropy for calling scripts as separate processes.

We could instead modify the tests so that they call the scripts as separate processes, which would be much more similar to their intended usage.

I would rather we don't do that. Testing CLIs through function calls is standard practice, and has the benefit of forcing a consistent and testable design.

@paverett, are you able to open a separate PR with just this bug fix ?

I'm not sure that's the best option. @eerovaher brings up a good point. The intended usage of a script is to run it on the command line so why should we not test it like its intended usage. It isn't a bug with the script just how we are calling it in the test. If we change the test then there is no problem.

eerovaher · 2024-03-05T15:56:08Z

astropy/io/fits/tests/test_core.py

+        def add_data_and_flush(hdul):
+            hdul[0].data = np.arange(0, 1000, dtype="int64")
+            hdul.insert(1, fits.ImageHDU())
+            hdul.flush()


It's not obvious introducing a helper function like this is the correct thing to do.

eerovaher · 2024-03-05T15:59:04Z

astropy/units/tests/test_format.py

+    print(string)
    with pytest.raises(ValueError):
-        print(string)


Why is this test printing anything at all?

I'm not entirely sure, I can remove it.

I think calling the __str__() or __repr__() methods should trigger the desired error without having to call the entire print() function.

Alright ill try out the repr() method on these tests.

I think the point here was that in old versions of pytest, one would not see what string actually went wrong. I think the print statement can safely be removed (and I'm pretty sure str() and repr() are tested elsewhere).

Yes, for some tests like the one above it would print just the enumeration of parameters like test_unit_grammar[strings9-unit9], so the actual string would add a bit of convenience, but for single parametrisation even that should not be necessary.

eerovaher · 2024-03-05T16:02:08Z

astropy/io/fits/tests/test_core.py

+        with open(self.data("test0.fits"), "rb") as handle:
+            with pytest.raises(ValueError):
                with fits.open(handle, mode="ostream") as _:


It might be better to use a single with-statement for all these context managers.

WilliamJamieson

The failing tests are due to a revealed bug. These changes should fix that.

WilliamJamieson · 2024-03-05T20:41:20Z

astropy/modeling/tests/test_input.py

-            g1 = models.Gaussian1D(
-                [10.2, 10], mean=[3, 3.2], stddev=[0.23, 0.2], n_models=2
-            )
-            y1 = g1(self.x1, model_set_axis=False)
            _ = fitter(g1, self.x1, y1)


Suggested change

_ = fitter(g1, self.x1, y1)

fitter(g1, self.x1, y1)

There is no use in even invoking the assignment here

Without the assignment, wouldn't this be removed by https://docs.astral.sh/ruff/rules/useless-expression/?

It does not trigger the issue when I run ruff locally with these changes.

WilliamJamieson · 2024-03-05T20:44:33Z

astropy/modeling/tests/test_input.py

+            theta=[0, 0],
+            n_models=2,
+        )
+        z = g2(self.x.flatten(), self.y.flatten())


Suggested change

z = g2(self.x.flatten(), self.y.flatten())

x = (self.x.flatten(), self.x.flatten())

y = (self.y.flatten(), self.y.flatten())

z = g2(x, y)

There is a bug in this test which is revealed by these changes. This should fix that bug. The message value will have to change.

WilliamJamieson · 2024-03-05T20:45:38Z

astropy/modeling/tests/test_input.py

@@ -235,17 +235,17 @@ def test_nonlinear_lsqt_Nset_2d(self, fitter):
            r"Input argument .* does not have the correct dimensions in .* for a model"
            r" set with .*"
        )


Suggested change

)

MESSAGE = r"Non-linear fitters can only fit one data set at a time"

Replace the whole message here.

WilliamJamieson · 2024-03-05T20:46:03Z

astropy/modeling/tests/test_input.py

-                theta=[0, 0],
-                n_models=2,
-            )
-            z = g2(self.x.flatten(), self.y.flatten())
            _ = fitter(g2, self.x, self.y, z)


Suggested change

_ = fitter(g2, self.x, self.y, z)

fitter(g2, x, y, z)

WilliamJamieson · 2024-03-07T04:25:11Z

It is very good to enforce PT012 because apparently we had tests that seem to have been written assuming that if there are multiple lines in a with-block that can raise an error then pytest.raises() checks all of them, when in fact it only checks the first error and the following lines in the block never get executed at all.

I agree. This type of issue also points to us considering including the test code within the code coverage as the test files should theoretically have 100% coverage if we include all our various CI configurations. I think a coverage measurement may have also pointed out these areas.

taldcroft

Fine for io.ascii and time.

nstarman · 2024-03-18T19:58:59Z

astropy/convolution/tests/test_kernel_class.py

+        if len(opt) == 1:
+            a = opt[0]
+            with pytest.raises(TypeError, match=r".* allowed .*"):
+                kernel(a, array=x)
+        else:
+            a, b = opt
+            with pytest.raises(TypeError, match=r".* allowed .*"):
+                kernel(a, b, array=x)


Would this work?

Suggested change

if len(opt) == 1:

a = opt[0]

with pytest.raises(TypeError, match=r".* allowed .*"):

kernel(a, array=x)

else:

a, b = opt

with pytest.raises(TypeError, match=r".* allowed .*"):

kernel(a, b, array=x)

with pytest.raises(TypeError, match=r".* allowed .*"):

kernel(*opt, array=x)

Yes, that's much better, thanks!

eerovaher · 2024-03-19T15:46:00Z

astropy/convolution/tests/test_convolve_fft.py

            # while a good idea, this approach did not work; it actually writes to disk
            # arr = np.memmap('file.np', mode='w+', shape=(512, 512, 512), dtype=complex)
            # this just allocates the memory but never touches it; it's better:


It looks to me like these comments should be moved too.

eerovaher · 2024-03-19T15:46:54Z

astropy/convolution/tests/test_kernel_class.py

@@ -45,7 +45,20 @@
    AiryDisk2DKernel,
    Ring2DKernel,
 ]
-
+KERNEL_TYPES_WITH_ARGS = [


If this is parametrization for a single test then why is it written out as a module-level constant?

I will remove the constant and use it in the parameterize statement.

eerovaher · 2024-03-19T15:47:31Z

astropy/coordinates/tests/test_angles.py

        Latitude(lon)

+    lon = Longitude(10, "deg")


Why is lon being defined again?

eerovaher · 2024-03-19T15:50:47Z

astropy/cosmology/_io/tests/test_latex.py

+        assert (
+            exc_info.type is ValueError
+            and "format must be 'ascii.latex'" in exc_info.value.args[0]
+            or exc_info.type is IORegistryError
+            and "No writer defined for format" in exc_info.value.args[0]
+        )


This looks very suspicious. Surely we know which error the code raises?

It can raise either error. Removing one or the other from the with statment produces failing tests. Maybe there is a better way to combine them in the with raises statement but I haven't been able to come up with one.

It might be the case that the test needs to be redesigned, but that task is beyond the scope of this pull request.

eerovaher · 2024-03-19T15:54:01Z

astropy/cosmology/flrw/tests/test_base.py

@@ -1019,8 +1019,6 @@ class HASOde0SubClass(cosmo_cls):
                def __init__(self, Ode0):
                    pass

-            _COSMOLOGY_CLASSES.pop(HASOde0SubClass.__qualname__, None)


If you want to ensure _COSMOLOGY_CLASSES gets restored after the test then the best way to achieve that is with a yield fixture.

eerovaher · 2024-03-19T16:07:10Z

astropy/units/tests/test_format.py

@@ -48,8 +48,8 @@ def test_unit_grammar(strings, unit):
    "string", ["sin( /pixel /s)", "mag(mag)", "dB(dB(mW))", "dex()"]
 )
 def test_unit_grammar_fail(string):
+    string.__repr__()


What is this for?

I thought we discussed to using the repr method instead of print, here: #16158 (comment)

If it is not necessary I can remove it.

It is no longer necessary

Yes, remove it.

eerovaher · 2024-03-19T16:08:15Z

astropy/units/tests/test_format.py

    with pytest.raises(ValueError), warnings.catch_warnings():
-        # ct, dex also raise warnings - irrelevant here.
-        warnings.simplefilter("ignore")


Why can't you just have nested with-blocks?

eerovaher · 2024-03-19T16:09:22Z

astropy/units/tests/test_quantity_non_ufuncs.py

@@ -2243,7 +2243,6 @@ def test_slogdet(self):
        # TODO: Could be supported if we had a natural logarithm unit.
        with pytest.raises(TypeError):
            logdet = np.linalg.slogdet(self.q)
-            assert hasattr(logdet, "unit")


Doesn't this removal change the test?

Don't see how hasattr could ever raise TypeError on the first parameter, so it shouldn't – but slodget is returning a tuple, so that assert seems out of scope anyway.

And if the np.linalg.slogdet() call raises an error then logdet isn't defined anyways. So the deletion of the line is indeed correct.

eerovaher · 2024-03-19T16:10:28Z

astropy/utils/tests/test_data.py

+    report_length = 1024
+    real_length = 1023
    with pytest.raises(urllib.error.ContentTooShortError):
-        report_length = 1024
-        real_length = 1023
        download_file(TESTURL, cache=False)


Do report_length and real_length even do anything?

Yes, removing them breaks the test. Seems like they are used up above in MockURL class.

It is very surprising that just setting local variables like that actually has an effect on the tests, but fixing this would go beyond the scope of this pull request.

eerovaher · 2024-03-19T16:11:22Z

astropy/wcs/tests/test_wcs.py

+    def init_wcs_and_alter(hdr):
+        w = wcs.WCS(hdr, _do_set=False)
+        w.all_pix2world([[536.0, 894.0]], 0)


Is the helper function needed?

No just had to flip flop the with statements.

eerovaher · 2024-03-19T16:15:17Z

I can see that in many cases the code I commented on is a rewrite of already existing code, I'm just not convinced that the existing code is always worth rewriting.

dhomeier · 2024-03-19T18:46:37Z

astropy/io/ascii/tests/test_c_reader.py

+    text = "A\tB\tC\nN\tS\tN\n4\tb\ta"  # C column contains non-numeric data
    with pytest.raises(ValueError) as e:


Suggested change

text = "A\tB\tC\nN\tS\tN\n4\tb\ta" # C column contains non-numeric data

with pytest.raises(ValueError) as e:

# C column contains non-numeric data

with pytest.raises(ValueError) as e:

read_rdb("A\tB\tC\nN\tS\tN\n4\tb\ta")

Agreed; temporary variables are unnecessary here, and so is forcing the line split with trailing comma – the comment could better be placed on a separate line (be it inside or outside the context, dunno how picky PT012 is there).

dhomeier · 2024-03-19T19:15:34Z

astropy/io/fits/tests/test_core.py

+        with (
+            pytest.raises(ValueError),
+            open(self.data("test0.fits")) as handle,
+            fits.open(handle) as _,
+        ):


So is the nested with construct disallowed by PT012 (cf. also https://github.com/astropy/astropy/pull/16158/files#r1530684773)?
I was not familiar with that form of using with; while it is kind of neat, it does not make the nested nature very clear and breaks symmetry with the reading operations that are not raising.

It is allowed to nest once with one statement inside the inner statement. Once a second nesting statement is added it complains.

Allowed

with pytest.raises(ValueError), open(self.data("test0.fits")) as handle: with fits.open(handle) as _: pass

Not Allowed

with pytest.raises(ValueError): with open(self.data("test0.fits")) as handle: with fits.open(handle) as _: pass

As for these changes, @eerovaher thought they might be better with a single with statement. I thought it made sense and made the change. #16158 (comment)

My suggestion might not have been a good one in this case. When I had a quick look at the tests I saw a bunch of nested with-statements and thought that it might be good to rewrite them as a single multi-line with-statement (which is new syntax in Python 3.10). However, the tests here are checking that some specific with-statements themselves are raising errors and having nested with-statements makes that clearer. So now I would recommend rewriting the nested with-statements if and only if the error being checked does not come from one of them.

Yes, there seem to be some open issues on Ruff handling those cases right, and the flake8 rule only lists the one-level nesting, not making clear whether this should apply recursively. Whatever flake8 says, the rationale for not missing an exception from the intended statement (here the fits.open(handle)) does not apply even for 3 or more context levels, because that statement simply could not be reached and thus not tested if any of the outer withs already failed.

dhomeier · 2024-03-19T20:39:53Z

astropy/convolution/tests/test_kernel_class.py

@@ -622,22 +635,11 @@ def test_kernel2d_initialization(self):
        with pytest.raises(TypeError):
            Kernel2D()

-    def test_array_keyword_not_allowed(self):
+    @pytest.mark.parametrize(["kernel", "opt"], KERNEL_TYPES_WITH_ARGS)
+    def test_array_keyword_not_allowed(self, kernel, opt):
        """
        Regression test for issue #10439
        """
        x = np.ones([10, 10])
        with pytest.raises(TypeError, match=r".* allowed .*"):


Suggested change

with pytest.raises(TypeError, match=r".* allowed .*"):

with pytest.raises(TypeError, match=r"Array argument not allowed for kernel.*"):

seems GitHub is not keeping up with current code version at all today; but if this is still valid, might as well put a fuller message here for clarity.

mhvk

Some inline comments (somehow seem to have started a review, so some are a bit old)

mhvk · 2024-03-12T20:45:58Z

astropy/units/tests/test_format.py

+    print(string)
    with pytest.raises(ValueError):
-        print(string)


I think the point here was that in old versions of pytest, one would not see what string actually went wrong. I think the print statement can safely be removed (and I'm pretty sure str() and repr() are tested elsewhere).

mhvk · 2024-03-20T13:10:11Z

astropy/units/tests/test_format.py

@@ -48,8 +48,8 @@ def test_unit_grammar(strings, unit):
    "string", ["sin( /pixel /s)", "mag(mag)", "dB(dB(mW))", "dex()"]
 )
 def test_unit_grammar_fail(string):
+    string.__repr__()


It is no longer necessary

mhvk · 2024-03-21T14:02:42Z

astropy/units/tests/test_format.py

    with pytest.raises(ValueError), warnings.catch_warnings():
-        # ct, dex also raise warnings - irrelevant here.
-        warnings.simplefilter("ignore")


As long as it is inside catch_warnings - otherwise the filter does not get reset.

eerovaher · 2024-04-15T20:52:04Z

Different astropy sub-packages have different maintainers, so if a pull request edits many sub-packages then it needs approvals from many people. If there are too many people involved then it can easily happen that everybody thinks that someone else is reviewing, in which case no one does. It looks to me like that's exactly what is happening here.

It would be good to get the updates merged because this pull request is fixing many mistakes in tests, but the best way to achieve that might be to open separate pull requests for the separate sub-packages. Then it would be clear whose responsibility it is to review each such pull request.

pllim · 2024-04-15T21:01:01Z

Looks like @WilliamJamieson requested changes so maybe he can at least re-review?

dhomeier · 2024-04-15T21:20:30Z

Different astropy sub-packages have different maintainers, so if a pull request edits many sub-packages then it needs approvals from many people. If there are too many people involved then it can easily happen that everybody thinks that someone else is reviewing, in which case no one does. It looks to me like that's exactly what is happening here.

Not sure if they expect that the reviews are already done by others, as the review scope should be within the respective sub-packages, but some of their maintainers simply don't have that much time (and I know of some who have already ceded code ownership because they felt overwhelmed by the Ruff PR reviews). Here we seem to only have explicit approval for io.ascii and time, since finding the right test structure is non-trivial in many cases, but for those sub-packages where maintainers have already responded and suggested changes, I think it's worth somehow cherry-picking the commits here and try to get them merged; moving the remaining sub-packages' changes to separate PRs.

nden

modeling and wcs look good

nstarman

Cosmology also looks good. Thanks @paverett !

eerovaher

coordinates needs to be updated.

eerovaher · 2024-04-16T13:29:45Z

astropy/coordinates/tests/test_angles.py

        Longitude(lat)

+    lat = Latitude(10, "deg")


This line needlessly duplicates line 842.

Suggested change

lat = Latitude(10, "deg")

fix: ruff pt012 violations

fb732a3

paverett requested review from WilliamJamieson, nstarman, mcara, astrofrog, Cadair, larrybradley, taldcroft, a team, saimn and dhomeier as code owners March 5, 2024 01:26

github-actions bot added testing io.votable wcs io.fits time io.ascii nddata units coordinates modeling cosmology utils convolution visualization uncertainty utils.iers dev-automation labels Mar 5, 2024

pllim added this to the v6.1.0 milestone Mar 5, 2024

pllim added the no-changelog-entry-needed label Mar 5, 2024

fix: tests not passing

8bcf4a6

nstarman reviewed Mar 5, 2024

View reviewed changes

eerovaher reviewed Mar 5, 2024

View reviewed changes

WilliamJamieson requested changes Mar 5, 2024

View reviewed changes

taldcroft approved these changes Mar 12, 2024

View reviewed changes

fix: broken tests and formatting

16bc69b

nstarman reviewed Mar 18, 2024

View reviewed changes

scripts output and simplify kernel test

6d9d6ef

eerovaher reviewed Mar 19, 2024

View reviewed changes

dhomeier reviewed Mar 19, 2024

View reviewed changes

mhvk reviewed Mar 21, 2024

View reviewed changes

fixing scripts tests, rearranging with contexts

d24ec43

astrofrog modified the milestones: v6.1.0, v7.0.0 Apr 4, 2024

nden approved these changes Apr 16, 2024

View reviewed changes

nstarman approved these changes Apr 16, 2024

View reviewed changes

eerovaher requested changes Apr 16, 2024

View reviewed changes

dhomeier mentioned this pull request Apr 19, 2024

ZodiPy pyOpenSci/software-submission#161

Closed

32 tasks

-        z = g2(self.x.flatten(), self.y.flatten())
+        x = (self.x.flatten(), self.x.flatten())
+        y = (self.y.flatten(), self.y.flatten())
+        z = g2(x, y)

	)
	MESSAGE = r"Non-linear fitters can only fit one data set at a time"

		text = "A\tB\tC\nN\tS\tN\n4\tb\ta" # C column contains non-numeric data
		with pytest.raises(ValueError) as e:

	with pytest.raises(TypeError, match=r".* allowed .*"):
	with pytest.raises(TypeError, match=r"Array argument not allowed for kernel.*"):

fix: ruff pt012 violations #16158

Are you sure you want to change the base?

fix: ruff pt012 violations #16158

Conversation

paverett commented Mar 5, 2024

Description

github-actions bot commented Mar 5, 2024

nstarman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paverett Mar 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eerovaher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paverett Mar 7, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhomeier Mar 19, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilliamJamieson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilliamJamieson Mar 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WilliamJamieson commented Mar 7, 2024

taldcroft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paverett Mar 7, 2024 •

edited

paverett Mar 7, 2024 •

edited

dhomeier Mar 19, 2024 •

edited

WilliamJamieson Mar 5, 2024 •

edited