Implement Ellipse Minor Axis  as Diameter

Following the discussions in #363, #349, this PR aims to implement the diameter of axon using minor axis of ellipse.
DONE:

[x] Implement minor axis as an additional feature to compute the diameter of an axon, thickness of myelin, diameter of axon_myelin
[x] In order to allow the user to chose minor axis or equivalent diameter while computing morphometrics as its diameter, user can manually set the boolen variable ellipse to be True or False.
[x] Made the necessary changes in 04-compute-morphometrics.ipynb notebook file, allowing the user to set his choice for diameter computation.
[x] Added comprehensive tests to test this new feature.
[x] Implement the similar behaviour in FSLeyes plugin, where the user is prompted to set his preferred choice of diameter either as equivalent diameter or ellipse minor axis.  Opened seperate issue for this see #432  and it will be dealt in seperate PR.
[x] Add documentation for this feature in notebook 04-morphometrics_extraction.ipynb
[x] Add a flag to select the shape of the axons
[x] Documentation: Add literature for axon shape (circle and ellipse)
[x] Add cli tests for the axon shape -a flag

What are the main contributions of this PR?

Implements Ellipse Minor axis as an additional way to compute morphometrics
For generating morphometrics via CLI, adds an flag -a to select the shape of axon. Usage: Refer to the docs for usage.
Updated the RTD

NOTE: - The default behaviour is set as equivalent diameter(circle) for measuring morphometrics. However, if the user wants to consider the axon as an "oblong ellipse", the user can set ellipse boolean variable to True.
Fix #363, #349

As discussed in the meeting, to validate this new feature,  I tested it on a good TEM mask, that is,  located in our default TEM model. Manual measurements were done using the protocol as mentioned in this comment. 4 manual measurements were done so that we can compare it with our new feature ( ellipse minor axis) and current method of calculating diamter (equivalent diameter).
Since this is my first time working with Fiji, I am including the mask on which I did manual measurements. So, if either of @mariehbourget  / @jcohenadad  could check the correctness of it superficially, that would be great.
.
The instances which were manually measured are 15, 16, 33, 40.

Here are the diameter measurements for 4 labels [15, 16, 33, 40] .
Manual Measurements :
|   |   |
-- | -- | -- | --
Instance | Axon Diameter | Myelin Thickness | Axon + Myelin Diameter
15 | 0.45361796 | 0.07566632 | 0.62155792
16 | 0.4111002 | 0.08243008 | 0.61844272
33 | 0.53261188 | 0.08286904 | 0.7177232
40 | 0.3493508 | 0.0708 | 0.51692732
Ellipse Minor Axis
| axon_diam | myelin_thickness
-- | -- | --
15 | 0.46739465 | 0.085082158
16 | 0.443414718 | 0.084947787
33 | 0.54835999 | 0.093597934
40 | 0.364738882 | 0.079313725
Equivalent Diameter
| axon_diam | myelin_thickness
-- | -- | --
15 | 0.566857219 | 0.092142902
16 | 0.565692604 | 0.087108396
33 | 0.634559572 | 0.091233045
40 | 0.400306314 | 0.090037793
Looking at the results, when ellipse minor axis is used to measure diameter, the results seems close to the manual measured values of the labels as compared to using equivalent diameter.

Since this is my first time working with Fiji, I am including the mask on which I did manual measurements. So, if either of @mariehbourget / @jcohenadad could check the correctness of it superficially, that would be great.

@vs74, yes the measurements are correct, and they are matching more closely with the minor axis method, which was expected, thanks!
We should also add a test for this new ellipse feature, similar to this one.

Added tests for this new feature. Anyone wants to give this PR a shot, please feel free to review it. Also,  I've added a couple of tests and changes in compute_morphometrics.py and launch_morphometrics_computation.py, so if we include changes in FSLeyes as well, this PR would be too huge to review.  I suggest, we open a separate PR to brainstorm for the FSLeyes, the design choices regarding how we should display to the user about using either ellipse_minor_axis  or equivalent diameter to compute the diameter.

This PR seems to be to merge between the branch vs74/ellipse_minor_axis on Vasudev fork, to the corresponding branch in ADS (and not to master in ADS). I think this is why it includes older commits and can be merged automatically.
To be able to review properly, we would have to first merge to the branch vs74/ellipse_minor_axis in ADS, then open another PR from this branch to master, no?

This PR seems to be to merge between the branch vs74/ellipse_minor_axis on Vasudev fork, to the corresponding branch in ADS (and not to master in ADS). I think this is why it includes older commits and can be merged automatically.
To be able to review properly, we would have to first merge to the branch vs74/ellipse_minor_axis in ADS, then open another PR from this branch to master, no?

No, we can change the base to be master on the PR page. I just did so.

@vs74 if a PR is ready to review pls make sure to remove the "[WIP]" in the title-- also: in general, do not use WIP, but use draft mode-- see our guidelines

This PR is ready for review.

This PR is ready for review.

Starting to review now.+

On naming it circle_approx in the code; yeah I'm quite certain we need to rename this, because now in the morphometrics Jupyter Notebook, we are instead setting the variable ellipse=true, so there's a lack of consistency between the API and the internal code here with the naming I think.

And actually, that functionality is broken too:

Nevermind for the previous error; I think there was a problem between conda env and Jupyter

The notebook 04-morphometrics_extraction.ipynb  is 1.16 MB, which suggests it probably includes binaries/images --> we need to clear that otherwise repos accumulates fat
--> meaning we should squash before merging

For the documentation concerned regarding this feature, I'm not sure what to add in the documentation becuase there are only changes done in the functions of  compute_morphometrics script and launch_morphometrics_computation script.
According to me, it would make more sense to have documentation for the CLI to generate morphometrics file (see PR #434 ), wherin I've added  -a --axonshape flag and documentation for that in PR #434 which will allow the user can select the desired axon_shape ( ellipse or circle).
So, I don't think documentation for this PR would be needed.

For the documentation concerned regarding this feature, I'm not sure what to add in the documentation becuase there are only changes done in the functions of  compute_morphometrics script and launch_morphometrics_computation script.
According to me, it would make more sense to have documentation for the CLI to generate morphometrics file (see PR #434 ), wherin I've added  -a --axonshape flag and documentation for that in PR #434 which will allow the user can select the desired axon_shape ( ellipse or circle).
So, I don't think documentation for this PR would be needed.

@vs74 PR #179 was merged, so you can now generate test images with ellipses using it.

@vs74 PR #179 was merged, so you can now generate test images with ellipses using it.

Shall we open a new PR for automatically generating synthetic images (  using #179 recently merged  into master) when axon shape is circle or we shall open another PR for the same? Currently, we are using a synthetic mask image in the test files hosted on OSF and testing the behaviour using the synthetic mask.

Shall we open a new PR for automatically generating synthetic images (  using #179 recently merged  into master) when axon shape is circle or we shall open another PR for the same? Currently, we are using a synthetic mask image in the test files hosted on OSF and testing the behaviour using the synthetic mask.

Shall we open a new PR for automatically generating synthetic images ( using #179 recently merged into master) when axon shape is circle or we shall open another PR for the same? Currently, we are using a synthetic mask image in the test files hosted on OSF and testing the behaviour using the synthetic mask.

No, not a new PR, but a new issue is warranted. Those masks are not directly related to this PR, so it doesn't belong here.

Shall we open a new PR for automatically generating synthetic images ( using #179 recently merged into master) when axon shape is circle or we shall open another PR for the same? Currently, we are using a synthetic mask image in the test files hosted on OSF and testing the behaviour using the synthetic mask.

No, not a new PR, but a new issue is warranted. Those masks are not directly related to this PR, so it doesn't belong here.

So, I tested added some tests which generate simulated axons ellipsoidal in shape with varying plane angle. The simulated axons are very much identical to the test image of simulated axons, which were circular except for the fact that they have varying plane angle, which makes the axon's shape ellipse.
    c.generate_axon(axon_radius=50, center=[100, 100], gratio=0.9, plane_angle=0)
    c.generate_axon(axon_radius=45, center=[200, 200], gratio=0.8, plane_angle=10)
    c.generate_axon(axon_radius=40, center=[300, 300], gratio=0.7, plane_angle=20)
    c.generate_axon(axon_radius=35, center=[400, 400], gratio=0.6, plane_angle=30)
    c.generate_axon(axon_radius=30, center=[520, 520], gratio=0.5, plane_angle=40)
    c.generate_axon(axon_radius=23, center=[650, 650], gratio=0.4, plane_angle=50)
    c.generate_axon(axon_radius=18, center=[750, 750], gratio=0.3, plane_angle=60)
    c.generate_axon(axon_radius=12, center=[830, 830], gratio=0.2, plane_angle=70)
    c.generate_axon(axon_radius=6, center=[920, 920], gratio=0.1, plane_angle=80)

    c.save()

The tests seem to fail for the axons with higher plane angle or in other words, the more the axons are ellipsoidal in shape, the measurements such as g_ratio and myelin_thickness are different from the ones it was generated on.
For example, for the axon instances [5, 6, 7, 8] there is a difference in the gratio and myelin_thickness measurements computed by the new feature and the measurements (gratio and myelin_thickness) of synthetic axons. Hence, the test cases fail.
I manually measured the axon_diameter, myelin_thickness and gratio for this test image following the protocol mentioned here. These manual measurements were much similar to the measurements generated by the new feature as compared to the measurements of synthetic axons.
So a question for @mathieuboudreau, shall I set the measurements (gratio and myelin_thickness) to the manual measurements for testing purposes?

So, I tested added some tests which generate simulated axons ellipsoidal in shape with varying plane angle. The simulated axons are very much identical to the test image of simulated axons, which were circular except for the fact that they have varying plane angle, which makes the axon's shape ellipse.
    c.generate_axon(axon_radius=50, center=[100, 100], gratio=0.9, plane_angle=0)
    c.generate_axon(axon_radius=45, center=[200, 200], gratio=0.8, plane_angle=10)
    c.generate_axon(axon_radius=40, center=[300, 300], gratio=0.7, plane_angle=20)
    c.generate_axon(axon_radius=35, center=[400, 400], gratio=0.6, plane_angle=30)
    c.generate_axon(axon_radius=30, center=[520, 520], gratio=0.5, plane_angle=40)
    c.generate_axon(axon_radius=23, center=[650, 650], gratio=0.4, plane_angle=50)
    c.generate_axon(axon_radius=18, center=[750, 750], gratio=0.3, plane_angle=60)
    c.generate_axon(axon_radius=12, center=[830, 830], gratio=0.2, plane_angle=70)
    c.generate_axon(axon_radius=6, center=[920, 920], gratio=0.1, plane_angle=80)

    c.save()

The tests seem to fail for the axons with higher plane angle or in other words, the more the axons are ellipsoidal in shape, the measurements such as g_ratio and myelin_thickness are different from the ones it was generated on.
For example, for the axon instances [5, 6, 7, 8] there is a difference in the gratio and myelin_thickness measurements computed by the new feature and the measurements (gratio and myelin_thickness) of synthetic axons. Hence, the test cases fail.
I manually measured the axon_diameter, myelin_thickness and gratio for this test image following the protocol mentioned here. These manual measurements were much similar to the measurements generated by the new feature as compared to the measurements of synthetic axons.
So a question for @mathieuboudreau, shall I set the measurements (gratio and myelin_thickness) to the manual measurements for testing purposes?

@mariehbourget, I've added some literature studies here, but these are not sufficient. If you happen to know literature where axon shape being circle or ellipse are used for morphometrics, feel free to contribute in the documentation of this PR or add them in the comments, which I will add on in the RTD. Thanks!!

@mariehbourget, I've added some literature studies here, but these are not sufficient. If you happen to know literature where axon shape being circle or ellipse are used for morphometrics, feel free to contribute in the documentation of this PR or add them in the comments, which I will add on in the RTD. Thanks!!

@mathieuboudreau did you get a chance to see my previous comment? I'd love to hear your thoughts on this.

@mathieuboudreau did you get a chance to see my previous comment? I'd love to hear your thoughts on this.

@vs74 I would recommend doing one image where only the gratio varies, and another where only the plane angle varies. The first case should definitely agree when getting morphometrics from the code, however I'm not convinced that the morphometrics calculation (well, the g-ratio value) is supposed to match for the case where the plane angle varies; in this case though, the major and minor axes should match with the ones I simulated (see here to see how they're calculated from the plane angle).

@mathieuboudreau correct me if I am wrong. Would the myelin thickness see here  be axon_radius * (1/gratio - 1)?

@mathieuboudreau correct me if I am wrong. Would the myelin thickness see here  be axon_radius * (1/gratio - 1)?

@mathieuboudreau correct me if I am wrong: wouldn't the myelin thickness see here  be axon_radius * (1/gratio - 1)?

@mathieuboudreau could you confirm the formula for myelin thickness so that I will proceed with the tests? Thanks!!

@mathieuboudreau correct me if I am wrong: wouldn't the myelin thickness see here  be axon_radius * (1/gratio - 1)?

@mathieuboudreau could you confirm the formula for myelin thickness so that I will proceed with the tests? Thanks!!

Hi Vasudev,
I'm currently working on a project that has a deadline tomorrow, so I will recheck the equation after that.
Thanks for the reminder,
Best,
Mathieu
--
Mathieu Boudreau, PhD
Research Fellow | Montreal Heart Institute
Software Developer | qMRLab and AxonDeepSeg
Lead Editor | MRM Highlightshttp://linkedin.com/company/mrm-highlights/https://www.linkedin.com/company/mrm-highlights/

From: Vasudev Sharma<EMAIL_ADDRESS>Sent: Thursday, January 28, 2021 8:45 AM
To: neuropoly/axondeepseg<EMAIL_ADDRESS>Cc: Mathieu Boudreau<EMAIL_ADDRESS>Mention<EMAIL_ADDRESS>Subject: Re: [neuropoly/axondeepseg] Implement Ellipse Minor Axis as Diameter (#399)
@mathieuboudreauhttps://github.com/mathieuboudreau correct me if I am wrong: wouldn't the myelin thickness see here https://github.com/neuropoly/axondeepseg/blob/28666b8be47889df24b6839cefd247e06de16102/AxonDeepSeg/visualization/simulate_axons.py#L72 be axon_radius * (1/gratio - 1)?
@mathieuboudreauhttps://github.com/mathieuboudreau could you confirm the formula for myelin thickness so that I will proceed with the tests? Thanks!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/neuropoly/axondeepseg/pull/399#issuecomment-769028585, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAK25ZLTOKKAL5UDULENNWLS4FL6BANCNFSM4T3QBT5A.

Hi Vasudev,
I'm currently working on a project that has a deadline tomorrow, so I will recheck the equation after that.
Thanks for the reminder,
Best,
Mathieu
--
Mathieu Boudreau, PhD
Research Fellow | Montreal Heart Institute
Software Developer | qMRLab and AxonDeepSeg
Lead Editor | MRM Highlightshttp://linkedin.com/company/mrm-highlights/https://www.linkedin.com/company/mrm-highlights/

From: Vasudev Sharma<EMAIL_ADDRESS>Sent: Thursday, January 28, 2021 8:45 AM
To: neuropoly/axondeepseg<EMAIL_ADDRESS>Cc: Mathieu Boudreau<EMAIL_ADDRESS>Mention<EMAIL_ADDRESS>Subject: Re: [neuropoly/axondeepseg] Implement Ellipse Minor Axis as Diameter (#399)
@mathieuboudreauhttps://github.com/mathieuboudreau correct me if I am wrong: wouldn't the myelin thickness see here https://github.com/neuropoly/axondeepseg/blob/28666b8be47889df24b6839cefd247e06de16102/AxonDeepSeg/visualization/simulate_axons.py#L72 be axon_radius * (1/gratio - 1)?
@mathieuboudreauhttps://github.com/mathieuboudreau could you confirm the formula for myelin thickness so that I will proceed with the tests? Thanks!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/neuropoly/axondeepseg/pull/399#issuecomment-769028585, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAK25ZLTOKKAL5UDULENNWLS4FL6BANCNFSM4T3QBT5A.

No worries, it's not that urgent.

No worries, it's not that urgent.

@mathieuboudreau correct me if I am wrong: wouldn't the myelin thickness see here  be axon_radius * (1/gratio - 1)?

@mathieuboudreau could you confirm the formula for myelin thickness so that I will proceed with the tests? Thanks!!

@vs74 I had the time to revisit this, and indeed it appears you are correct; there is an error in the equation:

I will create a branch to patch this and add tests that would have caught this in the first place. I'll let you know shortly if I encounter any other problems (conceptually or in the algorithm) prior to creating a pull request for you to review.

I will create a branch to patch this and add tests that would have caught this in the first place. I'll let you know shortly if I encounter any other problems (conceptually or in the algorithm) prior to creating a pull request for you to review.

Thank you @mathieuboudreau for revisiting this formula. I'm not sure if I as a contributor can review the PR of the repo. Is it possible to provide rights to contributors to review PR's? Also, note that the patched PR you will submit will result in failure of the tests because of PR #441, so we should deal with that first.

Ahh ok, whether or not you can approve the review, I'll ask you to check the PR since you're the one who discovered the bug!

An update on this PR: the tests seem to pass when the parameters "gratio", "radius" and "plane angle" are varied in isolation while keeping other parameters constant. However, as discussed before in the meeting, when all the parameters are varied, the tests seem to fail PR #399 (comment)

@mathieuboudreau did you get a chance to revisit this PR?

@vs74 I've finished doing as much digging as I could for now. I think I found one bug in your test (see comment above), but unsure if this was the sole reason for your test failing.
Here are my analytical derivations, and they all appear to be consistent with my code:

The only thing I had a minor concern about was the fact that myelin_major_radius requires the radius, plane_angle, and myelin_thickness values, because myelin_thickness already uses the plane angle to calculate an updated value, so I'm wondering if maybe there's some duplication in the logic here. To test this out, instead of varying all three parameters at a time, I've added 4 more tests in a branch that's a copy of this PR (in commit 4fff10af) to test the combinations of pairs of two parameters varying (plus one more for the case of zero plane angle). If one or some of these pass, that will give us more information into the conflicting parameters and could help debug this. TBD.

Here are my analytical derivations, and they all appear to be consistent with my code:

so beautiful (and rare) to see derivations on paper these days 😍

@vs74 your "single parameter varying" tests appear to be sensitive to the values given. i.e., I changed the parameters of your test test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies to the following:

And after that the test fails,

Which is super odd actually, because those parameters are all the same as the first set of parameters of your test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse test, which didn't fail at myelin_thickness check but at the gratio check,

and I don't know why that would be, even if there was a bug in my simulation code... ???
At this point I'm stuck again and out of ideas of where to look to easily fix the state of this issue. I think we may want to go back to the drawing board and start fresh with baby steps, slowly building the functions/features one feature/test at a time :/

@vs74 your "single parameter varying" tests appear to be sensitive to the values given. i.e., I changed the parameters of your test test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies to the following:

I didn't test the single parameter tests extensively with a wider range of values, so that could be the reason for the failure of the tests. Perhaps I got lucky with my previous numbers.

Which is super odd actually, because those parameters are all the same as the first set of parameters of your test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse test, which didn't fail at myelin_thickness check but at the gratio check,

Yes, it is weird. I need to revisit the tests again and I'll try to figure out the cause of this problem.
If it doesn't work out, then you are right that we have to take baby steps to figure out the cause of failure of the tests. I'll let you know once I have looked at it.
Thanks @mathieuboudreau  for looking into it.

Coverage increased (+0.05%) to 82.85% when pulling b2f77432303eed0e39872ec32ffecdfcc954f5be on vs74:vs74/ellipse_minor_axis into e4ffb28afe118566ec2dc5bba7e2bb859d55a0c6 on neuropoly:master.

@mathieuboudreau I revisited this PR and simulated your case where you mentioned that the tests in this PR were sensitive to the gratio values https://github.com/neuropoly/axondeepseg/pull/399#issuecomment-806222808
UNIT TEST (@mathieuboudreau tried)
    @pytest.mark.unit
    def test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies(self):

        # varying g ratio
        self.image_sim_ellipse = SimulateAxons()

        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[115, 90], gratio=0.9, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[200, 200], gratio=0.8, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[305, 305], gratio=0.7, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[410, 410], gratio=0.6, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[510, 520], gratio=0.5, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[650, 650], gratio=0.4, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[800, 800], gratio=0.3, plane_angle=0)

        self.image_sim_ellipse.save(self.image_sim_ellipse_path)

        gratio_sim = np.array([
                                0.9,
                                0.8,
                                0.7,
                                0.6,
                                0.5,
                                0.4,
                                0.3,
                                ])

        axon_diam_sim = np.array([
                                100,
                                100,
                                100,
                                100,
                                100,
                                100,
                                100,
                                ])

        myelin_thickness_sim = (axon_diam_sim / 2) * (1/gratio_sim - 1)

        # Read paths and compute axon/myelin masks
        pred = imageio_imread(self.image_sim_ellipse_path)
        pred_axon = pred > 200
        pred_myelin = np.logical_and(pred >= 50, pred <= 200)

        # compute axon morphometrics
        stats_array = get_axon_morphometrics(pred_axon, str(self.image_sim_ellipse_path.parent), im_myelin=pred_myelin, axon_shape=self.axon_shape)

        for ii in range(0,len(gratio_sim)):

            assert stats_array[ii]['gratio'] == pytest.approx(gratio_sim[ii], rel=0.1)
            assert stats_array[ii]['axon_diam'] == pytest.approx(axon_diam_sim[ii], rel=0.1)
            assert stats_array[ii]['myelin_thickness'] == pytest.approx(myelin_thickness_sim[ii], rel=0.1)

Here is the simulated image of the gratio values you tested with

Could it possible due to overlapping axons on the simulated image the tests are failing? I tried axon_radius ranging from 5-38, where the axonmyelin objects were not overlapping, and the tests passed. However, when the axonmyelin objects were overlapping the test cases were failing, that is for axon_radius > 41.
Note The tests often fail when the axonmyelin objects overalps in the simulated image. This is true for not only test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies, but also for other tests as well.
Let me know what do you think.

i would recommend to avoid overlap as it creates an indeterminacy on the true solution

Could it be possible that due to overlapping axons on the simulated image the tests are failing?

Yes, that's very likely at least one cause of it. I'd recommend we remove every other axon (axon #2, #4, etc) and try again to see if the results of the larger axons agree better. This way, we'll still cover the same range, but will avoid overlapping. Thanks for checking this!

Could it be possible that due to overlapping axons on the simulated image the tests are failing?

Yes, that's very likely at least one cause of it. I'd recommend we remove every other axon (axon #2, #4, etc) and try again to see if the results of the larger axons agree better. This way, we'll still cover the same range, but will avoid overlapping. Thanks for checking this!

Test 1
I commented axon # 4 and # 6 with the same axon_radius = 50 and plane_angle = 0 for the simulated image given below

 Commented unit test
@pytest.mark.unit
    def test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies(self):

        # varying g ratio
        self.image_sim_ellipse = SimulateAxons()

        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[115, 90], gratio=0.9, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[200, 200], gratio=0.8, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[305, 305], gratio=0.7, plane_angle=0)
        # self.image_sim_ellipse.generate_axon(axon_radius=50, center=[410, 410], gratio=0.6, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[510, 520], gratio=0.5, plane_angle=0)
        # self.image_sim_ellipse.generate_axon(axon_radius=50, center=[650, 650], gratio=0.4, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[800, 800], gratio=0.3, plane_angle=0)

        self.image_sim_ellipse.save(self.image_sim_ellipse_path)

        gratio_sim = np.array([
                                0.9,
                                0.8,
                                0.7,
                                #0.6,
                                0.5,
                                #0.4,
                                0.3,
                                ])

        axon_diam_sim = np.array([
                                100,
                                100,
                                #100,
                                100,
                                100,
                                #100,
                                100,
                                ])

        myelin_thickness_sim = (axon_diam_sim / 2) * (1/gratio_sim - 1)

        # Read paths and compute axon/myelin masks
        pred = imageio_imread(self.image_sim_ellipse_path)
        pred_axon = pred > 200
        pred_myelin = np.logical_and(pred >= 50, pred <= 200)

        # compute axon morphometrics
        stats_array = get_axon_morphometrics(pred_axon, str(self.image_sim_ellipse_path.parent), im_myelin=pred_myelin, axon_shape=self.axon_shape)

        for ii in range(0,len(gratio_sim)):

            assert stats_array[ii]['gratio'] == pytest.approx(gratio_sim[ii], rel=0.1)
            assert stats_array[ii]['axon_diam'] == pytest.approx(axon_diam_sim[ii], rel=0.1)
            assert stats_array[ii]['myelin_thickness'] == pytest.approx(myelin_thickness_sim[ii], rel=0.1)

Test 2
I even tried with mostly large axons by commenting axon # 2 and axon # 3 for the simulated image given below, and the tests passed.

 Unit test with mostly large axon myelin instances
   @pytest.mark.unit
    def test_get_axon_morphometrics_with_myelin_mask_simulated_axons_with_axon_as_ellipse_gratio_varies(self):

        # varying g ratio
        self.image_sim_ellipse = SimulateAxons()

        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[115, 90], gratio=0.9, plane_angle=0)
        #self.image_sim_ellipse.generate_axon(axon_radius=50, center=[200, 200], gratio=0.8, plane_angle=0)
        # self.image_sim_ellipse.generate_axon(axon_radius=50, center=[305, 305], gratio=0.7, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[220, 220], gratio=0.6, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[360, 360], gratio=0.5, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[550, 550], gratio=0.4, plane_angle=0)
        self.image_sim_ellipse.generate_axon(axon_radius=50, center=[800, 800], gratio=0.3, plane_angle=0)

        self.image_sim_ellipse.save(self.image_sim_ellipse_path)

        gratio_sim = np.array([
                                0.9,
                                #0.8,
                                #0.7,
                                0.6,
                                0.5,
                                0.4,
                                0.3,
                                ])

        axon_diam_sim = np.array([
                                100,
                                #100,
                                #100,
                                100,
                                100,
                                100,
                                100,
                                ])

        myelin_thickness_sim = (axon_diam_sim / 2) * (1/gratio_sim - 1)

        # Read paths and compute axon/myelin masks
        pred = imageio_imread(self.image_sim_ellipse_path)
        pred_axon = pred > 200
        pred_myelin = np.logical_and(pred >= 50, pred <= 200)

        # compute axon morphometrics
        stats_array = get_axon_morphometrics(pred_axon, str(self.image_sim_ellipse_path.parent), im_myelin=pred_myelin, axon_shape=self.axon_shape)

        for ii in range(0,len(gratio_sim)):

            assert stats_array[ii]['gratio'] == pytest.approx(gratio_sim[ii], rel=0.1)
            assert stats_array[ii]['axon_diam'] == pytest.approx(axon_diam_sim[ii], rel=0.1)
            assert stats_array[ii]['myelin_thickness'] == pytest.approx(myelin_thickness_sim[ii], rel=0.1)

Ananlysis:
I guess because of the overlapping axon myelin instances the tests were failing. All the tests for this PR are passing when the simulated image has no overlapping axonmyelin instances. We have tested on only varying the plane angle, only varying gratio, only varying axon_radius, and lastly varying all the three parameters (axon_radius, plane_angle, and gratio).
@mathieuboudreau do you want to try out some additional tests on this feature? If not, then I will go ahead with cleaning this PR and would then ask for final reivew.

This PR is up for a reivew.

Excellent debugging @vs74 – thank you so much for discovering this bug in our test datasets! I'll review ASAP.

@vs74 did you merge your changes to the test to avoid overlap? I don't see them there; there's always 8-9 axons instead of your proposed 5. Also, why are the tests currently passing if these are note included in the tests? This is the last thing I need to see before merging.

@vs74 did you merge your changes to the test to avoid overlap? I don't see them there; there's always 8-9 axons instead of your proposed 5. Also, why are the tests currently passing if these are note included in the tests? This is the last thing I need to see before merging.

In order to avoid overlap, for the test only varying gratio, I switched to the old parameter values axon_radius=25, plane_angle=60(8-9 axons) with varying gratio values for the axon, which were passing earlier too. However, when you tested on this PR with axon_radius=50, plane_angle=0 (https://github.com/neuropoly/axondeepseg/pull/399#issuecomment-806222808) on this tests itself, it failed because the axon_radius was big enough to cause overlap.
Thus, I switched to old values axon_radius=25, plane_angle=60(8-9 axons) for the test only varying gratio. Also, you tried testing it for plane_angle=0 for this test which falied, and this plane_angle value wasn't making sense because this test was specifically designed to test elliptical-shaped axon myelin simulated image.
Lastly, I changed the center parameter of other tests ( I guess for 1-2 tests) while keeping the simulated axons to 8-9 axons, which were preventing the tests from failing due to the overlapping nature of axons.
I have two suggestions:

You can try locally testing the test snippet for the particular value you tested on https://github.com/neuropoly/axondeepseg/pull/399#issuecomment-850205734, it will fail. However, when you will try test for non-overlapping axons https://github.com/neuropoly/axondeepseg/pull/399#issuecomment-850486505, it will pass.

Or, I can change the test (only varying gratio) to 5 axons with axon_radius = 50 (non-overlapping) but with different plane_angle

@vs74 please go with option number 2; I don't have my old laptop capable of running ADS on hand at the moment, so I wouldn't be able to do number 1 before tomorrow.

@vs74 please go with option number 2; I don't have my old laptop capable of running ADS on hand at the moment, so I wouldn't be able to do number 1 before tomorrow.

Sure, I'll do the changes for the test after the meeting.

@mathieuboudreau I updated the test  only varying gratio to Test 2 (non-overlapping axons with large radius) https://github.com/neuropoly/axondeepseg/pull/399#issuecomment-850486505 with just plane angle== 20 instead of plane_angle==0 because the tests are for elliptical simulated image.

@mathieuboudreau I updated the test only varying gratio to Test 2 (non-overlapping axons with large radius) #399 (comment) with just plane angle== 20 instead of plane_angle==0 because the tests are for elliptical simulated image.

The plane angle were 10 before you changed them to 20, not 0, so not sure if this modification changed anything.


[Feature] Function to calculate Allison-Foster Index

The Allison-Foster is a polarization measure within a single group defined as:
$$s(x; c) = \mu_{H}(x; c) - \mu_{L}(x; c)$$
where:

$s(x; c)$ - Allison-Foster index,
$\mu_{H}(x; c)$ - mean of the values above (H) group median value,
$\mu_{L}(x; c)$ - mean of the values below (L) group median value.

Implement is a function that takes list-like objects with responses within a group. The function should return three elements:

Allison-Foster index,
$\mu_{H}(x; c)$
$\mu_{L}(x; c)$.

Write docstring in a numpy-doc style: https://numpydoc.readthedocs.io/en/latest/format.html#sections
Write a simple functional test for the function using sample data.
Steps:

(Optional) Read paper: Allison RA, Foster JE. Measuring health inequality using qualitative data. J Health Econ. 2004 May;23(3):505-24. doi: 10.1016/j.jhealeco.2003.10.006. PMID: 15120468. Especially page 514.
Define a function as a Python script (it could now be a stand-alone script).
Write docstring.
Write a functional test (it could be the code that was used for function development).

PR https://github.com/hs3city/ordinal-scale-stats-py/pull/20


is this being updated?

As in, are you adding new entries etc.?

also my question. This looks like an old "burner-email" list of another repo, which has never been updated in years?
Link: https://github.com/findie/burner-email-providers/blob/master/emails.txt

@tbba the base list is around that, but this one is having more than them, the idea is to keep this updated and cover most of them which are available out there.

How can we request to add more disposable email domains to the list?

@Arindam-Bitcanny feel free to raise a PR if you want to add them yourself, else you can provide the source/list for such domains and i'll get them added.


PHPUnit 10 Shift

This pull request contains changes for upgrading to PHPUnit 10 automated by the PHPUnit 10 Shift.
Before merging, you need to:

Checkout the shift-92180 branch
Review all pull request comments for additional changes
Run composer update (if the scripts fail, add --no-scripts)
Run your tests suite: vendor/bin/phpunit

If there were changes you felt could have been automated, please reply to the follow-up email with your feedback or on Twitter.

:warning: PHPUnit 10 requires PHP version 8.1 or higher. Be sure to verify you met this requirement by running php -v in your test environment.

:warning: PHPUnit 10 has made several changes to the configuration file. After running composer update, you may run vendor/bin/phpunit --migrate-configuration to have PHPUnit upgrade your configuration file.

:information_source: PHPUnit has documented using return types for test cases and data providers since PHPUnit 8. In an effort to modernize your test suite, Shift added a return type of void to your test cases and a return type of array to your data providers.
Shift understands developers have different preferences when it comes to type hints. If you do not wish to add return types, you may undo this change by running git revert cde4632b.

:warning: PHPUnit 10 now requires any base test class to have a TestCase suffix. In addition, this class should be defined as abstract. If you have a custom base test class, you should ensure it meets these new requirements.

:information_source: PHPUnit has documented declaring test classes as final since PHPUnit 8. In an effort to modernize your test suite, Shift has declared your test classes as final.
Shift understands developers have different preferences when it comes to using final. If you do not wish to declare your test classes as final, you may undo this change by running git revert 40697f55.

:alembic: This Shift is still being refined. Please report any issues or suggestions to<EMAIL_ADDRESS>Your feedback is what helps improve the experience for everyone.


Add Flakes support

This change adds basic support for flakes.
It works but I'm unsure about the approach:
I've added an optional flake parameter, when set to true it will consider the value of nixos_config to be the name of the attribute to lookup in the nixosConfiguration output of the flake located in the same folder as the Terraform config.

I'm using this, works like a charm :)

I just hit an issue where this doesn't work if the flake is in a sub-folder of the repo. Because it tries to instantiate from ./. and that doesn't contain the .git folder.


encoding: Don't unnecessarily escape everything

Apparently all the base64 and md5 blocks have been doing encodeURIComponent on the input before encoding/hashing, so they've been returning the wrong thing for quite some time.

A little concerned that this is technically a breaking change


feat: support react 19

Description

Related Issue: Fixes # (issue_number)
Changes

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Documentation update

Checklist

[ ] I have performed a self-review of my own code.
[ ] My code is commented, particularly in hard-to-understand areas.
[ ] I have made corresponding changes to the documentation.
[ ] My changes generate no new warnings.
[ ] I have added tests that prove my fix is effective or that my feature works.
[ ] New and existing unit tests pass locally with my changes.
[ ] Any dependent changes have been merged and published in downstream modules.

Further Comments

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.48%. Comparing base (37980bc) to head (2941002).

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #95   +/-   ##
=======================================
  Coverage   81.48%   81.48%
=======================================
  Files          11       11
  Lines         243      243
  Branches       49       49
=======================================
  Hits          198      198
  Misses         43       43
  Partials        2        2

Components
Coverage Δ

overlay-kit
81.48% <ø> (ø)


Cannot install on MacStudio (Apple Silicon M1 Max)

I am getting the following error when trying to install via npm:
npm ERR! ../src/netcdf4-async.cpp:3:10: fatal error: 'netcdf_meta.h' file not found
npm ERR! #include <netcdf_meta.h>
npm ERR!          ^~~~~~~~~~~~~~~
npm ERR! 1 error generated.

Installed netcdf 4.9.2_1 via Homebrew. Version of node-gyp is v9.4.0.

Late to the party but for anyone else installing via homebrew check your exports (e.g. in ~/.zshrc)
# homebrew
export PATH=$PATH:/opt/homebrew/bin
export LDFLAGS=$LDFLAGS:/opt/homebrew/lib
export CPPFLAGS=$CPPFLAGS:/opt/homebrew/include


docs(CONTRIBUTING): Refined contrubuting guidelines.

Hey,
I've tried to refine contributing guidelines a bit, hope you'll like the changes. I've also moved it to the root of the project so it's not GitHub-specific and it's easier to find. I've found some useful notes about local development there, but at first I didn't really think to look for these in .github directory.
Maybe we could add a link in the Readme?
Cheers

That's brilliant! Thanks so much for helping out! I really appreciate that!


clicking on "Add Graph" does nothing.

I am following the getting started doc, on Mac, I am using prometheus-1.5.0.darwin-amd64
when I click "Add Graph" nothing happens.
in chrome's inspect I see
Uncaught ReferenceError: Handlebars is not defined
    at graph.js:6

that line is
Handlebars.registerHelper('pathPrefix', function() { return PATH_PREFIX; });

Hm, this project doesn't use Handlebars, so I'm not sure what you're looking at. The file you reference (graph.js) doesn't have that code, either.

I just re-did everything and here I screenshot for you

here is the location of the file if it helps debugging.

That's prometheus, not promdash. You should open this issue over there.


SortableTable chevron does not change direction when row is expanded

Setup

Rancher version: v2.6-head
Browser type & version: Firefox 98.0.2

Describe the bug
When the SortableTable component has expandable sub-rows the chevron button that is used to expand the row does not change direction.
To Reproduce

Install CIS benchmark
Run a scan and view the results
Expand one of the rows

Result
The chevron will not update
Expected Result
The chevron should change direction indicating that the row is expanded
Screenshots
https://user-images.githubusercontent.com/40806497/160457276-b575c3b2-2310-4335-8b23-f7877c4665b0.mp4
Additional context

Detailed Description

Context

Reopened on v2.6.5-rc6
Steps to reproduce:

Bring up HA Rancher 2.6.5-rc6
Install CIS Benchmark into local cluster with default settings
Run and view Scan with default settings
Click the chevron next to State once the scan completes (still in fixed position when expanded)


Fix for #42

Checks if statusCode is defined before access it, otherwise, logs the error stacktrace. Handling response error crashes #42

@longstone @shernshiou giving it a little more look at the debug, it seems not handling the res properly gave me a little light on something else. Why is the library calling _setWebhook on stop when it doesn't need to? On start there's a check to call that, but on stop it's being called everytime with "" which I think might be the reason behind the crash.

@shernshiou any ideas on this? Do you think you can merge this pull request?

@kaminoo I will merge this code this week ya.. Sorry for the delay.

@kaminoo I have merge can u check again whether it works as expected?

@shernshiou thanks! checking this will take a couple of days, since I'll have to update the production server where this error occurs. It has happened in development, but it's hard to come by (probably why you didn't see it). I'll deploy it in the next days probably and let you know if it works as expected or not.


Support OTEL over HTTP for Access Logs

          @zirain any plans on doing otel over http?

writing a grpc service just to get the access log looks like overkill to me...
Originally posted by @pkit in https://github.com/envoyproxy/gateway/issues/701#issuecomment-2116284807

In most of the cases, otel-collector is the best choice as a backend/transit.

Maybe I can try this working!

otel-collector support both http and grpc, see the configuration here, you can use it as intermediary agent send other exporters(e.g. another otel backend but http protocol).

Yeah. Ok. Thanks.

When I looked through the documentation, it seems that the configuration in OpenTelemetryAccessLogConfig.common_config does not support us to choose HTTP protocol for export. ref

You can extend it to support HTTP, just as OpenTelemetry tracer do.

您可以扩展它以支持 HTTP，就像 OpenTelemetry 跟踪器一样。

I started working on this and I don't quite understand it. Do we need to enhance and extend this in Envoy?

您可以扩展它以支持 HTTP，就像 OpenTelemetry 跟踪器一样。

I started working on this and I don't quite understand it. Do we need to enhance and extend this in Envoy?

bingo!

您可以扩展它以支持 HTTP，就像 OpenTelemetry 跟踪器一样。

I started working on this and I don't quite understand it. Do we need to enhance and extend this in Envoy?

bingo!

This seems to be a challenge for me, because I am not familiar with cpp and find it difficult to see this activity.


Caret up/down

Describe the requested icon
Caret up / down. Frequently used for sorting.
Screenshots
Just an example:

Added caret-up-down in v2.0.0


request to be re-judged

Your pull request seems good, but must be checked by a human maintainer for the following reason(s):

This contains potentially hazardous tags that need manual review.

Please be patient until a maintainer has time to look over your contribution.
This is an automated response, beep boop.
if you think there is a mistake, contact a maintainer

25d940fbe463ae0a74726339f0b071cd9bcbf0d0

<title>Chameera Dulanga</title>
not sure if you meant to leave this considering it's not repeated anywhere else in the file, but i think that you forgot to change this. if not just tell me and i'll merge.

This request's code has changed since approval. Re-Judging contents...
Your pull request seems good, but must be checked by a human maintainer for the following reason(s):

This contains potentially hazardous tags that need manual review.

Please be patient until a maintainer has time to look over your contribution.
This is an automated response, beep boop.
if you think there is a mistake, contact a maintainer

ba342a11319a7092d329958c3b70dcdaaf5a49f1


Enable localization of SketchPad

We don't have a lot of strings in the UI, so this should be relatively easy to maintain, with a few benefits:

More accessible to developers around the world
Show off Flutter's i18n capabilities
New contributors to DartPad (starting with localizing)

cc @mosuem for FYI, if we want to try using the upcoming package:message for this. From an off-line discussion, he thought using it would be reasonable (and I think it would be a good way to dogfood our upcoming APIs).
(the flutter web UI for dartpad is at https://github.com/dart-lang/dart-pad/tree/main/pkgs/sketch_pad)


Support :target :bundle

The new bundle target of clojurescript compiler breaks doo/cljs-test-runner. The problem is that the file named by :output-to in compiler config is an intermediate file to be consumed by webpack, usually named index.js. It is not ready te be evaled as normal JS. It's the file generated by webpack we're interested in, usually named main.js.
Figwheel solved this problem by introducing another config var, :final-output-to.
I forked doo myself and added a :output-to options to doo-opts, that overrides the compiler config one if present.
Change is rather small, you can find it here https://github.com/ingesolvoll/doo.
That repo also contains change necessary for it to be consumed as a git dep in deps.edn.

Hey, thank you for teaching me about the new bundle feature, it sounds really useful and definitely something that should be supported by cljs-test-runner.
Since you've already made the fork and we can git dep it I suggest consuming your fork for now. If I need to make further changes I can always fork the fork, so no pressure to maintain or develop further.

Okay, there's a develop branch now with updated deps and your fork of doo. Does it work okay for you? You'll have to use it as a git dep for now until I release it.

Thanks for the quick response! Will test, hopefully today. This obviously won't work for users that don't use tools.deps, but I guess that's ok for the short term.

Sorry to leave you hanging on this, my build broke down for reasons that were too complex to spend time on. So task has been put on hold, and I will have to try to find time in between to sort it out. Hopefully soon.

No problem! I have an infinite todo list to be getting on with (not counting actual work work todo list), so this can stick around until we're both ready to address these things. It's not like the tool is unusable right now 😄 (I hope)

I'm trying this out, but get either ReferenceError: doo is not defined with -x chrome-headless or ReferenceError: window is not defined from node (I guess I need jsdom there, but not sure how to set all that up).
I created a doo.edn with {:output-to "out/main.js"} and passed -D doo.edn to my clojure -Atest -c build.edn command (hopefully got all that right).
I'm happy to help further debug this as I would love to get my doo-based CLJS tests working under :target :bundle w/ tools-deps.
Here is what my deps.edn contains:
{:deps {org.clojure/clojurescript {:mvn/version "1.10.773"}
        fluree/alphabase {:mvn/version "3.2.1"}
        org.bouncycastle/bcprov-jdk15on {:mvn/version "1.65.01"}
        com.lambdaworks/scrypt {:mvn/version "1.4.0"}}
 :aliases {:test {:extra-paths ["test"]
                  :extra-deps {olical/cljs-test-runner
                               {:git/url "https://github.com/Olical/cljs-test-runner"
                                :sha "7daff9a9a62cb8824bfccfb6ef7a429ecbac3e5c"}}
                  :main-opts ["-m" "cljs-test-runner.main"]}}}

build.edn:
{:main fluree.crypto
 :output-to "out/index.js"
 :output-dir "out"
 :target :bundle
 :bundle-cmd {:none ["npx" "webpack" "out/index.js" "-o" "out/main.js" "--mode=development"]
              :default ["npx" "webpack" "out/index.js" "-o" "out/main.js"]}
 :closure-defines {cljs.core/*global* "window"}} ;; needed for advanced

Hmm that's odd although I'm 100% out of context right now. I'm afraid this isn't at the forefront of my mind right now, all of my energy is directed elsewhere. Thank you for testing it out and producing output, it should be valuable! I'll have to see if I can repro some time soon!

@Olical I managed to sort out my non-related issues now, so ran a test. First I tested with my existing setup, that works (forked doo, 3.7.0 of cljs-test-runner)
Then I tried without forked doo and this dependency for cljs-test-runner:
{:git/url "https://github.com/Olical/cljs-test-runner.git"
 :sha     "7daff9a9a62cb8824bfccfb6ef7a429ecbac3e5c"}

Getting an error, so I'm probably doing it wrong:
Can't find 'cljs-test-runner.main' as .class or .clj for lein run: please check the spelling.

Ok, so I finally got a successful test run. @cap10morgan , if this is useful for you, this is my setup:
An alias named :test-cljs with the following config
{:extra-paths ["test/clojure"]
                         :extra-deps  {olical/cljs-test-runner {:git/url "https://github.com/Olical/cljs-test-runner.git"
                                                                :sha     "7daff9a9a62cb8824bfccfb6ef7a429ecbac3e5c"}}
                         :main-opts   ["-m" "cljs-test-runner.main"
                                       "-D" "doo.cljs.edn"
                                       "-c" "test.cljs.edn"
                                       "-x" "chrome-headless"]}

doo.cljs.edn contains {:output-to "resources/public/js/main.js"}
test.cljs.edn contains
{:target     :bundle
 :output-to  "resources/public/js/main.out/index.js"
 :bundle-cmd {:none ["npx" "webpack" "--mode=development" "./resources/public/js/main.out/index.js" "-o" "./resources/public/js/main.js"]}
 :output-dir "resources/public/js/main.out"}

I run it with clj -A:test-cljs

Since we seem to be among the few people in the world that care about this, we should probably discuss the design choices for configuring doo. Right now I just check if there is an :output-to value in doo.edn, and let that override the one in compiler config. Is this a good design?
Figwheel-main has chosen to call the main.js file :final-output-to. Better for this case?

@ingesolvoll Thanks! I'm still not getting it to work, though. Is your working repo available somewhere? I think there's some other difference between my setup and yours that's breaking it.

Never mind, I got it working. 🎉
There are still a couple of 404 warnings output from the karma web server, but they seem to be a red herring. Looks like the thing that was actually tripping me up was having a :main entry in my test-build.edn (i.e. the file that gets passed to the -c arg).

I've been out of the loop on this, but is there anything that should be changed in cljs-test-runner? And are those changes kind of just convenience, so it's all doable without them but it would be nice to have some first class support?

I've been using this for some time now on several projects. It's working great. I'd be in favor of merging it as-is if it doesn't break anything else.

It's working, but only on projects based on deps.edn, as the patched doo isn't properly released as an artifact.

@Olical I finally got around to deploying this to clojars. Tried running it now and it seems to work.
Deployed doo:
https://clojars.org/ingesolvoll/doo/versions/0.2.0
Change needed in cljs-test-runner: https://github.com/ingesolvoll/cljs-test-runner/commit/2071d1fec183f04e3380cf0cb4d01496fb095348
This makes it ok for leiningen projects as well, although I didn't test it in leiningen yet. But since the maven-style dep works in deps.edn, I don't see how it wouldn't work.

Ohh nice! I've swapped over on develop if you'd like to try it there, I'll leave it on there as a note for now. I don't have a good cljs project to test it on these days :grimacing: I'm doing lots of backend Clojure and cloudformation atm, essentially zero CLJS so I worry that I won't be able to give it a proper check. We'll see!

https://github.com/Olical/cljs-test-runner/pull/37 Dep and doc
As I said I already tested this on my deps.edn setup. Maybe @cap10morgan could test as well?
Is there a quick and easy way to create a local maven repo jar for the runner so I can test on my lein setup with latest on develop?

I think mvn build? And thanks for the PR!

I'm just merging and syncing pom etc

Going to release a 3.8.0-SNAPSHOT

@Olical Excellent, I'll test that on leiningen when it arrives, then.

Okay, 3.8.0-SNAPSHOT is up

Can confirm that 3.8.0-SNAPSHOT works with both my leiningen and deps.edn setup

I'll get this released ASAP :smile:

@Olical 3.8.0-SNAPSHOT worked for me too. Time for a release? :)

Released! All still okay?

Sorry about the delay, I've been all over the place recently :grimacing:

Sorry about the delay, I've been all over the place recently 😬

You don't owe us anything. Super happy with the fix and the release :)

Yay! :tada: well I'll finally close this issue then, it took far too long considering the changes :sweat_smile: have a great day and let me know if you run into any issues!

@Olical Can you publish the release on GitHub too? Thanks!


e2e_replication mysql tests

MySQL e2e tests for Replication.

@itsmekumari MySQL tests should be added to https://github.com/data-integrations/database-delta-plugins as that repo contains MySQL replication plugin. https://github.com/data-integrations/datastream-delta-plugins is only used for Oracle replication.

@Vipinofficial11 @itsmekumari MySQL tests should be added to https://github.com/data-integrations/database-delta-plugins as that repo contains MySQL replication plugin. https://github.com/data-integrations/datastream-delta-plugins is only used for Oracle replication.

@sumitjnn Closing this PR, will raise a new PR in repo for MySQL and SQLserver replication plugins https://github.com/data-integrations/database-delta-plugins


Bot crashes on start

Describe the bug
The Bot crashes if u start it cause there is a new website popup he doesnt know yet.
Didnt know if its the same issue than #34 so I created a new issue
To Reproduce

start the bot
bot tries to login
crash

Expected behavior
Should work as smooth as before :)
Screenshots

Desktop (please complete the following information):

OS: Linux
Browser: Chrome

Additional context
From the logfile:

Started session: 2021-04-21 08:34:06
User is not logged in yet.
COOKIES ACCEPTED.
Traceback (most recent call last):
File "c0rrrebot.py", line 95, in
main()
File "c0rrrebot.py", line 71, in main
session.login_using_facebook(email, password)
File "/home/tinder/Schreibtisch/TinderBotz/tinderbotz/session.py", line 144, in login_using_facebook
helper.login_by_facebook(email, password)
File "/home/tinder/Schreibtisch/TinderBotz/tinderbotz/helpers/login_helper.py", line 119, in login_by_facebook
loginbutton.click()
File "/home/tinder/.local/lib/python3.7/site-packages/selenium/webdriver/remote/webelement.py", line 80, in click
self._execute(Command.CLICK_ELEMENT)
File "/home/tinder/.local/lib/python3.7/site-packages/selenium/webdriver/remote/webelement.py", line 633, in _execute
return self._parent.execute(command, params)
File "/home/tinder/.local/lib/python3.7/site-packages/selenium/webdriver/remote/webdriver.py", line 321, in execute
self.error_handler.check_response(response)
File "/home/tinder/.local/lib/python3.7/site-packages/selenium/webdriver/remote/errorhandler.py", line 242, in check_response
raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.ElementClickInterceptedException: Message: element click intercepted: Element ... is not clickable at point (248, 201). Other element would receive the click: ...

hello @c0rrre ,
The cookies popup on the Facebook login is now handled, so pulling the latest version should do the trick.
Thanks for contributing to Tinderbotz and help making it bugfree. :)
If this resolved your problem, please close this issue.
Greetings,
Fred

Hallo @frederikme ,
Thanks for the fast bugfix and the awesome piece of software


Update some targets, add requirements file

The changes themselves look good but Travis isn't reporting anything for this.

The changes themselves look good but Travis isn't reporting anything for this.

travis had a planned outage yesterday , which left this build in an undefined state i think.... i'll commit amend


Use slice-dst until custom DSTs land

I started reading about DSTs and stumbled upon the slice-dst crate. With it it should be possible to implement Iri (and IriRef) as DSTs right now. The caveat is that we can't use a Vec for the Buf types but I wrote an example that demonstrates how a mutable buffer can be implemented with slice-dst (cf. CAD97/pointer-utils#70). Actually, most changes would be to re-write the replace function.

Sounds nice, I'll look into it.

It is actually a good theoretical idea, but I don't think it would be a good idea in our case. The header is not stored in the fat pointer but in the slice itself. This means that functions such as Iri::from_str or even Iri::new are impossible to implement since they borrow the user input slice, over which we have no control (so we cannot insert our header into it).

Okay, then I had a misunderstanding. I thought your goal was to implement Iri and IriRef as slice-like types like str and [u8] which both have no constructors. In fact, there is only a static constructor for &'static str namely the string literals. And a similar iri! macro already exists.
I think the question is where to put ParsedIriRef. The benefit I see storing it inline like in my example for slice-dst is that Iri (and IriRef) would really work like str. The life time of Iri could be removed and traits like AsRef<Iri>, Borrow<Iri>, ToOwned<Owned = IriBuf> and so on could easily be implemented.
Storing ParsedIriRef on stack and point to the 'data slice' is the current implementation. As can be seen, there are some difficulties with lifetimes, borrowing Iri from IriBuf without cloning ParsedIriRef and inconsistency with standard traits, e.g. Borrow and ToOwned.
Another idea that comes to my mind would be to use references for ParsedIriRef and the 'data slice' which doesn't solve the problems of the current implementation as far as I can see.  Maybe I'm wrong.
The last (and worst) would be to make Iri only a wrapper around str that ensures that the underlying data is a valid Iri. This way methods like Iri::from_str could still work but on each operation you'd have to parse the str again.

I thought your goal was to implement Iri and IriRef as slice-like types like str and [u8] which both have no constructors

I'm not sure what you mean by "slice-like types". My idea is to implement Iri and IriRef as DSTs, where a reference &Iri is represented as a fat pointer (ParsedIriRef, *const u8). Indeed this is much like the str DST for which &str is stored as a fat pointer (usize, *const u8). This implementation would allow a low cost conversion between &Iri and &str where we only would have to replace the metadata of the fat pointer, without touching to the actual data.

I think the question is where to put ParsedIriRef

Exactly, and there are only two solution: either we store it in the metadata (ideal) or in the data (not ideal). With slice-dst, the metadata is still usize and the header (in our case ParsedIriRef) has to be stored in the data, which means that a conversion between &Iri and &str is not possible anymore. So until custom DSTs are shipped, the only solution I see is the current one: wrapping &'a str inside a Iri<'a> that also contains our header (extending the metadata).
I agree that this is not an ideal solution (for the reasons you mentioned), but I think it is the best we can do for now.

Exactly, and there are only two solution: either we store it in the metadata (ideal) or in the data (not ideal).

Okay I had to read up on DSTs again to get that. So the idea is to store ParsedIriRef as part of the fat pointer that points to an Iri. Well, given that ParsedIriRef takes 12 words of space (96 bytes on a 64bit machine) I don't think that's a good idea. Given that references are Copy that would mean to always copy 104 bytes (including the data pointer) when we pass around a &Iri. I don't feel that that follows Rust's principle of making expensive operations explicit. In contrast, storing ParsedIriRef inline with the data on the heap would prevent copying the metadata.

With slice-dst, the metadata is still usize and the header (in our case ParsedIriRef) has to be stored in the data, which means that a conversion between &Iri and &str is not possible anymore.

I think this is only half true. You can still return an &str by pointing to the 'data' slice stored on the heap (cf. from_utf8(). However, the other way around wouldn't work. You can't just write 96 bytes in front of the memory of a &str. Especially, as &'static str are normally stored in the read-only memory section of a binary.

Well, given that ParsedIriRef takes 12 words of space (96 bytes on a 64bit machine) I don't think that's a good idea.

That's already what is happening with Iri and IriRef though. Both types are Copy. But there are ways to optimize this. For instance we could use usize instead of Option<usize> in ParsedIriRef with a special value for None, that would reduce the size of the metadata to 64 bytes. We can even use u32 or even u16 instead of usize (this may even be a parameter of Iri) since Iris are generally small. That would go down to 22 bytes total. If you know that the Iri will not be larger than 254 bits, you can specify u8 and go down to 15 bytes total. I really don't think it is that bad. And if you really don't want to copy this much data, you can use &&Iri instead.
I was more or less waiting for custom DSTs before introducing those optimizations, but I could do that now.

However, the other way around wouldn't work

And in my experience this way is the most important: you generally create IRIs by borrowing a string or byte slice and rarely go the other way unless you want to display them.

Note that I may have been a bit optimistic about alignment considerations here. In the last case, we will most probably end up with 16 bytes and not 15, with a 1 byte padding after ParsedIriRef so that it is correctly aligned.

I should add that If you don't care about the &str -> &Iri conversion, then I'm not sure using DSTs are really useful at all. You can directly use IriBuf since the conversion &IriBuf -> &str already exists. It is already a "store the header in the data" solution (and bonus, it is mutable) :smile:
Now that I think of it, what could be interesting is an equivalent of IriBuf stored on the stack instead of the heap maybe backed with smallvec or constvec. The actual storage type could be a parameter of IriBuf with a default to Vec.

Okay, alright I hadn't the memory optimization in mind. On that regard:

But there are ways to optimize this. For instance we could use usize instead of Option<usize> ... with a special value for None, ...

Instead of doing such an implicit contract, I recommend leveraging Rust's type system with non-zero integers. They are built for exact this use case where:
use std::mem::size_of;
assert_eq!(size_of::<Option<core::num::NonZeroU16>>(), size_of::<u16>());

I'm closing this now, but we can continue the discussion on the metadata optimization in #10.


[App Service] Add support for creating a webapp joined to a vnet

Description

Adds --vnet and --subnet arguments to az webapp create to allow creating webapps that are joined to a virtual network
Also refactors az webapp vnet-integration add to use the new API for integrating vnets with web apps
Testing Guide

az webapp create ... --subnet <subnet_name> --vnet <vnet_name>
az webapp vent-integration add ...
History Notes

This checklist is used to make sure that common guidelines for a pull request are followed.

[X] The PR title and description has followed the guideline in Submitting Pull Requests.

[X] I adhere to the Command Guidelines.

[X] I adhere to the Error Handling Guidelines.

App Service


cannot move config file because it is not found

This step is done to ensure that any previous nvim configurations don't conflict with the new configuration. I've added instructions explicitly stating that this needs to be done only if a config file exists. Let me know if the issue persists. Closing the issue for now. 👍


Time travel does not work properly with Symbols

When I'm using Symbols as action type it doesn't show in the log window but the inspector shows an empty value.

sorry now I am reading the docs that Symbols are not recommended .

The reason and the solution is described in https://github.com/zalmoxisus/redux-devtools-extension/issues/142.


Node process never exits if connection.end() isn't called

Steps:

run a program like the example in the doco:

'use strict'
var mysql = require('promise-mysql');
var connection;

mysql.createConnection({
    host: process.env.DBURL,
    port: process.env.DBPORT,
    user: process.env.DBUSER,
    password: process.env.DBPASS,
    database: process.env.DBNAME
}).then(function(conn){
    connection = conn
    return connection.query('select 1;')
}).then(function(rows){
    //connection.end() // doco example doesn't show calling end()
    console.log(rows)
})

Expected:
The query executes, data is available in the then(), the data is logged out to the console and the Node process exits.
Actual:
Most of that happens but the Node process never exits. I have different behaviour depending on the version of Node on my Linux machine:
v6.10.3:
I can't control+c the process, I need to kill the pid. Also, nothing is written to the console until you kill the process then it all appears.
v7.10.0:
The output is written to the console as it should (as the code is executed presumably) and I can control+c to end Node but the process still never exits on its own.
If you uncomment that connection.end() call, it all works as it should.
There's not necessaryily anything wrong with node-promise-mysql, it's just my fault for not thinking to call .end(). Maybe the doco just needs updating to indicate that calling end() is required to save other people trying to track down this issue. It wasn't particularly obvious to me but then I am a n00b Node programmer.
Happy to create a PR if you'd like. Thanks for the great module BTW.

Alternatively you can also close the connection after the query is done but then you can't use that same connection further down the chain. Might not suit everyone.
'use strict'
var mysql = require('promise-mysql');

mysql.createConnection({
    host: process.env.DBURL,
    port: process.env.DBPORT,
    user: process.env.DBUSER,
    password: process.env.DBPASS,
    database: process.env.DBNAME
}).then(function(conn){
    let result = conn.query('select 1;')
    conn.end()
    return result
}).then(function(rows){
    console.log(rows)
})

Thanks for letting me know! If you want to make a PR to update the docs
then go ahead, otherwise I'll try and update it myself we at some point.
On Wed, 24 May 2017, 07:10 Tom Saleeba<EMAIL_ADDRESS>wrote:

Alternatively you can also close the connection after the query is done
but then you can't use that same connection further down the chain. Might
not suit everyone.
'use strict'
var mysql = require('promise-mysql');
mysql.createConnection({
host: process.env.DBURL,
port: process.env.DBPORT,
user: process.env.DBUSER,
password: process.env.DBPASS,
database: process.env.DBNAME
}).then(function(conn){
let result = conn.query('select 1;')
conn.end()
return result
}).then(function(rows){
console.log(rows)
})
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/lukeb-uk/node-promise-mysql/issues/51#issuecomment-303619283,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AEjy1BOgsKfaS2kJLjd3xuqzwOj3amC0ks5r87u1gaJpZM4NkmnO
.

PR created :+1:

Fixed in #52


Remove unused KVM (libvirt) support

This is a virsh-based implementation over ssh which is no longer used.

@gmcculloug this is great. Anything in particular to pay attention before merging?


Add Social Media Links to ContributorCard

This pull request enhances the ContributorCard component by adding optional social media links. Contributors can now include their LinkedIn and GitHub profiles, which will be displayed as icons in the card.
Changes Made

Modified ContributorCard.js to include social media links.
Added linkedinUrl and githubUrl as props to ContributorCard.
Integrated icons for LinkedIn and GitHub using responsive images based on color scheme.
Added links for Yasser as a test

Resolves #10


Remove Promise dependency for Knit core

Just use coroutines instead of promises. Light-weight and easier for users to understand. Promises a great, but a little overkill for what Knit is trying to accomplish on startup.
Another current issue is that promises can hide errors on Knit lifecycle methods, which have to be caught with :catch(). It is better to fail hard if an error comes up on startup.

This will force a major version increment.


For 1.13 Features: set Milestone to 1.13 and Base Branch to dev-1.13
Help editing and submitting pull requests:
https://kubernetes.io/docs/contribute/start/#improve-existing-content.
Help choosing which branch to use:
https://kubernetes.io/docs/contribute/start#choose-which-git-branch-to-use.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Thanks.
/lgtm
/approve

/assign @cody-clark

Hey, @chenrui333, sorry I saw this lgtm-ed before I saw your comments on the duplicate PR. Do you want me to revert this and continue the discussion?


Possible fix for #917

This should prevent constrain errors, one of side effects observed is that some messages might not get deleted in the first run. #917
Usage of two additional nested subqueries is extremely heavy here though... not sure if doing one select query and then using PHP to select ids to remove wouldn't be a lighter solution here.
PS. telegram_update query was broken because of this missing AND updated_at < \'%1$s\' statement for chat table - it didn't clean the table until I added that - @noplanman ?

Ok, I've looked into this and there is a super simple fix for this:
Reverse the direction of message deletion!
Then there are no constraint problems :tada:
@jacklul I've added a commit to this PR with just that tiny change, please test it to see if it also works for you.

@jacklul Have reverted my changes and left yours, which does the trick :wink:
Good to merge?

Actually I think the subquery is not required and will only impact performance, works on my end!

@jacklul Doesn't work for me, I get this:
Warning: PDO::query(): SQLSTATE[HY000]: General error: 1093 You can't specify target table 'message' for update in FROM clause in /vendor/longman/telegram-bot/src/Commands/AdminCommands/CleanupCommand.php on line 398


Cannot modify font-size via yml config file in text stamp style.

Describe the bug
Cannot modify font-size via yml config file in text stamp style.
To Reproduce
To reproduce, use the following yml config.
stamp-styles:
    default:
        type: text
        background: signature.png
        stamp-text: "Signed by %(signer)s\nTimestamp: %(ts)s"
        text-box-style:
            font-size: 32

Note that in this example background: signature.png works perfectly fine and changes the background according to the corresponding file.
Expected behavior
Expected output: the font is increased to 32 from the default value of 10.
Environment (please complete the following information):

OS: Windows 10
Python 3.8.9
PyHanko 0.12.1

I just realized that it automatically scales to the maximum available font based on the box size.


fix: new possible proto name

Checklist

[x] npm test passes
[x] tests and/or benchmarks are included
[x] commit message follows commit guidelines

Affected core subsystem(s)

grpc loader
Description of change

Type name of service that loaded by<EMAIL_ADDRESS>changed from Client to ServiceClient.

你来发 patch


How to install in Node-Red?

Hi @amsel77, for now, this SCD4x node is not listed in Node-RED's library, because I don't have a way to test it – I don't have a Sensirion SCD4x sensor.
You can still install this locally, and it might work. See Testing a node module locally.
Does that work for you?

Hi Ron,
many thanks for the answer.
I'm unfortunately still a node-red beginner.
The manual installation @Pi was successful but
with worrying messages about dependencies.
I would have to deal with the testing more closely,
but I have already installed an SCD41 and it works
with the test program provided by the manufacturer.
To enable listing in the Node-Red library and to support
your useful work for node-red community I would be
willing to send you an evaluation board (SCD41).
Just give me your postal address- if I can trust that it will
lead to the desired result...
Greetings to Prague
Jan
Burghausen/Germany.
Von: Ron Šmeral @.>
Gesendet: Freitag, 28. Januar 2022 22:42
An: rsmeral/node-red-contrib-scd4x @.>
Cc: amsel77 @.>; Mention @.>
Betreff: Re: [rsmeral/node-red-contrib-scd4x] How to install in Node-Red? (Issue #1)
Hi @amsel77 https://github.com/amsel77 , for now, this SCD4x node is not listed in Node-RED's library, because I don't have a way to test it – I don't have a Sensirion SCD4x sensor.
You can still install this locally, and it might work. See Testing a node module locally https://nodered.org/docs/creating-nodes/packaging#testing-a-node-module-locally .
Does that work for you?
—
Reply to this email directly, view it on GitHub https://github.com/rsmeral/node-red-contrib-scd4x/issues/1#issuecomment-1024664213 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AXNME2WNIYIJ33BIRA5G7QDUYMERBANCNFSM5MSVOQ6Q .
Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675  or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .
You are receiving this because you were mentioned.  https://github.com/notifications/beacon/AXNME2QYPQ2SOJR7QTMGE5TUYMERBA5CNFSM5MSVOQ62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHUJSFFI.gif Message ID: @.***>

Hi Jan, sorry for the delay. Did you have any luck getting the sensor to work in Node-RED in the meantime?
I really appreciate the offer to send me one board, but I'm afraid I can't accept it for now.
I will likely get one board for myself anyway though. I'll let you know if I manage to test the package and publish it.


Fix: Updating links in  aws-ecs installation docs

FIxed the broken links under Further Reading on this page

@Pranay105, the link is working fine, no? https://docs.appsmith.com/getting-started/setup/installation-guides/aws-ecs#further-reading 🤔


Recent version bump potentially breaks Azure Functions support

It looks like this recent commit may have broken support for Azure Functions, as in #23 we had to downgrade the versions to get it to work correctly.
It's this commit:
https://github.com/IEvangelist/azure-cosmos-dotnet-repository/commit/871b496bdc516396aec3fb92545c9bb15e4fa72d#diff-ad7a9c31b9a37579b3004efcd03d04fffed6be0fed1e3d8dfc61787b91ea41a0R48

Downgrading to 2.1.4 works for me as a workaround for my talk tomorrow. 😎
I know there are .NET 5.0 workers we can look into so you aren't dependent on the Functions host. Maybe I'll look into that.

What worker version are you targeting, this should work as these extension packages multi-target and support .NET Standard 2.0. That is supposed to be the glue that allows for this to function correctly.

Hi, I am having the same issue here.
I thought it was my environment but then I run the sample code for the Functions in this repo and got the same error. Tried on a different pc too but the same result.
Is there any workaround for this?

For now you could use version 2.1.4. I'm still looking into this issue as I believe it to be a problem with Azure Functions themselves. Also, this literally just happened: https://azure.microsoft.com/en-us/updates/general-availability-azure-functions-supports-net-5-in-production/

Closing due to inactivity and known work around. If using older rev of Azure Functions, rely on v2.1.4.


Ginodev

Added Linkedin Link

Hello @ginodev,
Thanks for contributing.
Unfortunately, there are some things wrong in your PR:

Please, don't change the version number, it will be done automatically when we'll release a new version
Don't modify the ps_versions_compliancy variable
Be careful to your indentation. We now use spaces, not tabs.
All the PRs need to be made on dev branch, not master

So, I have to close this PR. Could you redoing it dev branch considering my remarks?
Regards

Yes you are right!
I've download this version from core 1700-RC.0


...

...

Coverage remained the same at 100.0% when pulling f5d1ceed7bfa75cbab7e881db84c9d406ceadd94 on X-Bird:master into 988ca3555dc7384af3ddc6acf902067edd133f78 on vilic:master.


Open Container by Name feature does not search all containers

Preflight Checklist

[X] I have installed the latest version of Storage Explorer.
[X] I have checked existing resources, including the troubleshooting guide and the release notes.
[X] I have searched for similar issues.

Storage Explorer Version
1.35.0
Regression From
No response
Architecture
x64
Storage Explorer Build Number
20240810.1
Platform
All
OS Version
No response
Bug Description
When I search for a container using the Open Container by Name feature from the tree view, it seems like it will only search the containers loaded in the tree view. When I use the View All feature, I can search for all  of my containers.
Steps to Reproduce

Launch explorer
Go to Blob Containers node of a storage account
Select Open Container by Name
Search for container that exists

Actual Experience

Message is displayed: "No results were found"
Be scared shitless because you think your data is gone

Expected Experience
I expect Open Container by Name to search within all containers in your storage account, and not some subset.
Additional Context
We have hundreds of containers of which the names are basically GUIDs.

@marcelvb Open Container by Name does indeed search all containers in your account, not a subset. What search terms were you using, and what were the names of the containers you were expecting to see? Please note that it searches containers by prefix. This is due to a limitation of the List Containers API only searching by prefix. Anything more than that would require client-side functionality and has a detrimental effect on performance. Also check your casing. I'm not sure if prefix search is case sensitive.

Workaround that seems to do the job is to click on View All first.  After that Open Container by Name seems to work fine.

@ebudimlic Are you saying you also experience this issue? Can you share a GIF or video of you reproducing the issue? I am not able to reproduce on my end.


Fix to set alt text on the media item

Add a media item reference to AltTextController and set it before showing the view. Use that item's alt text to initialize the text view and make it first responder when the controller's view is shown. Update the media item's alt text when the user selects the "Add" button. Also remove unused outlets and commented out code.
Tested by starting a new post, adding an image, and selecting the "Add Description" option for that image. Confirmed that the text is stored in the media item by entering some text, selecting "Add", and then re-selecting the "Add Description" option to see the same text. Confirmed that selecting "Cancel" leaves the text unchanged. Completed the post and confirmed that the text appears in the alt tag for the image.

Thank you!


status effect source

Renamed LootContext.getSupplier to LootContext.getTable as well, legacy name

Squash pushed after fixing on github due to action failing for wrapper validation.

Re haykam: it appears your concern is on the handling of the old/preexistent effect. I have clarified that the old one is just removed.

The new JD is much better :+1:


Adding "Etag" header

Hello,
First, thank you for this middleware collection!
I'm wondering if, as stated on https://github.com/micheh/psr7-cache/blob/master/src/CacheUtil.php#L118-L121, it is a good idea to add the ETag header when we are returning some cached response?
What do you think? I can try to do a PR if you think it's a good idea.

I'm not sure about what do you mean.
In theory, if you're returning the ETag header, the middleware returns it too, because it returns the same headers of the cached response: https://github.com/middlewares/cache/blob/master/src/Cache.php#L48

No that's not what I meant, I was wondering if it sounds right to ALSO add the Etag header when we are adding the Last-Modified one in https://github.com/middlewares/cache/blob/master/src/Cache.php#L62.
It seems that the 2 headers should be added when caching is enabled, as per the docblock linked in my first comment.

My idea was the inner middleware (the route controller) returns a response with the ETag (if it's needed), because this is something specific to the current route. Because the response headers is saved in cache, and the method isNotModified can handle the ETag header of the request/cached response: https://github.com/micheh/psr7-cache/blob/master/src/CacheUtil.php#L264, I don't know why add an ETag header automatically.
Anyway, if you are convinced and want to do a PR to illustrate the idea, feel free.
Thank you

OK got it, I didn't see this in CacheUtil at first, so let's keep it like this, closing this now.


On mobile. front page defaults to 2 columns for color display change to 3

this still has 2 columns on my default safari iPhone view. should default to 3 or take up maximum space. (remove whitespace on right side)


Support SlimerJS

I have a bunch of tests that do not work in PhantomJS due to the very old version of WebKit they are employing. I'd like to have the option of switching to SlimerJS which implements the same API as PhantomJS, but runs JS against an up-to-date Firefox.
What would be a feasible approach to optionally switch to SlimerJS at runtime, assuming that all existing code would work exactly the same? The switch should probably be made transparent for the consumers of the grunt-lib-phantomjs dependency.
A first step could just be finding a good "switch" capability (a global setting in the Gruntfile or else) and making a proof of concept implementation.
Would that be in the scope of this project?

FYI, the latest release of this lib uses PhantomJS v2.1.1 which is the latest release that based on an updated version of WebKit (see here and here).
If we did the switch, this lib should be renamed to something else and need to depend on an installer of SlimerJS (if exist). So this issue is out of the scope of this project.
I suggest creating a generic lib that have options to choose whether you want to use PhantomJS or SlimerJS.
Closing this for now, feel free to ask if you have followup questions.


Backport "Attach Amberscript transcriptions as tracks"

Fixes #6179
Combines abfecd9b4cc8efab496ca961e7f195b6fb78f747, e4653e927e2d275401aa94cad2e67e4e51199c94 and
929bbc4273eacadb7a3231c129c1dacf8b2e90a4 into a single commit and points the changes at r/15.x instead of r/16.x.
Due to lack of access to Amberscript I did not test this., but at least there is a PR now which people with Amberscript can test.
Why is this aimed at legacy?
This intends to fix a bug that breaks Amberscript transcription.

I'm not per se against this PR, but in light of the recently passed proposal, changes to legacy branches have to be explained in order to prevent breaking changes (see https://groups.google.com/a/opencast.org/g/dev/c/VwYqk3vXdd0 and https://github.com/opencast/opencast/pull/6239).

We will use version 15.x in our institution for this semester and will not switch to version 16 during operation.
Our users use both amberscript or wishper for subtitling their videos.
And in our current 15 productive instance - especially for the use of wishper - we have switched from attachments to tracks for subtitles and thus want to ensure that the use of amberscript is also possible.
@mtneug: Is that enough justification for you?

@mtneug: Is that enough justification for you?

Not only my decision! Each PR is discussed shortly in the weekly Opencast dev meeting every Tuesday. I don't think we really reached a decision for this PR today. So further discussion will take place here.
Note that this change in our approach for legacy branches is new and we don't have a history of what "good" reasons for changes to legacy branches are. However, I tend to classify "Institution A wants feature X in legacy in order to use it now" as not a valid reason. Ultimately, we want to minimize changes such that adopters can trust releases of the legacy version to install without any problems.

Regardless of what we as an institution imagine to be a desirable feature for a still supported version, we would have expected that the switch from attachments to track for subtitles with the release of version 15.0 in December 2023 (sic!) would also guarantee the functionality of the translation services supported by Opencast, such as amberscript. Hence our wish to approve this PR. Or is this expectation exaggerated?

tl;dr: I think it's fine for this to go into r/15.x if, and only if, this gets tested properly.

Since I caused this change in policy as a consequence of bringing the results of the “Make updates easier/more stable” to a vote on list, let me chime in here.
I fully agree with Matthias that “I need it now while we don't want to update” isn't really a good argument in general. Also the “fix a bug that breaks Amberscript transcription” is just a half-truth since it still works exactly as it did before and Opencast can still use subtitles as attachments. The idea behind the policy change is to force someone who wants to push things into legacy to really think about if this is necessary and what possible complications are. We have seen problems in the recent past because we carelessly added/back-ported patches to legacy without much testing. If you think about it, the bug has now existed in a release for over 10 months without anyone complaining and we are about 1.5 months before EOL of Opencast 15.
That being said, if I would want to argue for this, I think that Opencast encourages using tracks, that the operation's documentation advertises it will do that, and then it silently doing something else would be a good argument. Combine this with the observation that this is a low risk patch which would likely just break the amberscript-attach-transcription operation if this doesn't work properly, I think it is fine for this to go into legacy.
But then looking at f3105b1, I see a good argument to not merge this as is:

Due to lack of access to Amberscript I did not test this.

We shouldn't push things into legacy hoping it's fine and occasionally breaking a release. Legacy should be really stable. That means that patches need to be properly tested before we merge them. This should be the case with develop, but becomes much more important the older the release branches become. Michael, since you are interested, maybe you can help with testing this.

I understand your arguments and will bow to the policy, especially as I don't have time to test the patch at the moment.

100% agree that this should not be merged without proper testing, which is exactly why I emphasized that I did not do that.

Also the “fix a bug that breaks Amberscript transcription” is just a half-truth since it still works exactly as it did before and Opencast can still use subtitles as attachments.

Sure, it still works like before, but the context around it changed. Opencast not only encourages subtitles as tracks since 15.0, but some parts of Opencast will straight up only work with subtitles if they are tracks, i.e. the editor. Therefore, I would consider this a bug that should have been solved as part of #4407.

The recent release of Opencast 15.12 is intended to be the final minor release for 15. Please retarget your PR or explain why it warrants another Opencast 15 release.

If you are testing this, @gregorydlogan, I posted a complete amberscript configuration (except for the api key) in pull request #6299

This caused problems when merging r/15.x into r/16.x. Given that this was a backport of something that's already in r/16.x my solution was to just use the current version that's in r/16.x.


[type:refactor] add issue template

Yeah,  I think grok-1 needs an issue template to avoid confused issue

LGTM 👍


Remove calls to (quit) from abcl commands

The --batch flag causes abcl to exit. The (quit) doesn't actually work there.

Closing this since the ABCL tests are wedged at the moment.


Issue 672 Pet Settings Dropdown

🔗 Issue

https://github.com/rubyforgood/pet-rescue/issues/672
✍ Description

Turns out it is a known issue with the responsive table that dropdowns get cut off (https://github.com/tabler/tabler/issues/1343#issuecomment-1308569481)
I tried the popper config solution but it led to undesirable behavior when scrolling so I opted for setting the min height to give room for the dropdown to be displayed.
📷 Screenshots/Demos

https://github.com/rubyforgood/pet-rescue/assets/51760107/534650b1-01bf-42f2-9b08-65ca117e4326

@all-contributors
please add @OliverLeighC  for code.

@OliverLeighC looks good! Thank you


Fix style of extensions on left and right pane

Fixes #550
Screenshot with changes:

Developer's Certificate of Origin 1.1
   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

Thx @ajbozarth also for helping in getting my clean script to properly work, that was the issue.
Only when overriding css selections worked for me, because when deleting those lines, my browser would pick up an old css file from notebook-scheduler-extension, which was still installed in my env


multiple tracks of annotations

have way to show multiple sets of annotations, e.g. stacked rows of them within bar
From conference call with EBI 19/07/19.

ok, got this now, not perfect but its a start
can make more specific issues for problems with the way this currently works


🐛 v3 prerelease: Biome won't start

VS Code version
1.92.2
Extension version
v2024.8.301002 (pre-release)
Biome version
0.3.3
Operating system

[ ] Windows
[X] macOS
[X] Linux

Description
Biome won't start.
Steps to reproduce
Not sure about steps to reproduce yet, but I am running inside a DevContainer. The DevContainer is running a linux image, and the host is on Mac M1.
Expected behavior
I expected Biome to start up and work.
Does this issue occur when using the CLI directly?
Not sure / Not applicable
Link to a minimal reproduction
No response
Logs
I do not see the "Biome Trace" category. I only see "Biome" there.
It shows this:
2024-09-06 08:11:57.971 [info] Biome extension activated
2024-09-06 08:11:58.918 [error] Failed to start Biome extension

https://github.com/biomejs/biome-vscode/issues/329 ? Same thing.

How is it the same thing? I'm receiving different things in my output window, right?

There's more than 1 Biome log.

It's weird. I only see one. Called Biome.

The extension spawns one session per project and one global session. The global session is named Biome LSP and the project sessions have the path of the project as a suffix

Not for me. Very weird. It seems it crashes before this?

There was an issue where the extension attempted to create a global session before creating a project session. If that failed for some reason, the rest wouldn't work. I believe https://github.com/biomejs/biome-vscode/commit/899efe17d31f7dc9866d9bb8ace13b7d64d03318 fixes that. Would you mind trying the latest pre-release (v2024.9.91939)?
I'll close in the meantime, but feel free to circle back if it does not fix the issue.


Removed dependency on Curb

The Curb gem limits this gem to MRI only because of the native bindings to Curl. Curb is only used for posting requests, so using Net::HTTP here is a better idea.

agreed!  reducing dependencies always == good

This is great! Thanks @TheKidCoder

Sorry for the delay. I've been missing github notifications.

Resolves #22