 I think this is too much effort for a minute use case, as
I don't think users will insert these values to query the library to see what
degenerate distributions look like.
Having to deal with the special cases reduces clarity and complicates the API
considerably, as it forces to contend with mathematical details difficult to convey
in a numerical library. As the limit α -> inf is approached, the pointwise
limit of the pdf and the cdf is zero, so these aren't pdfs and cdfs anymore. As the
limit β -> inf is approached, the continuous distribution degenerates to a discrete
one: P(X = 0) = 1.
I propose to remove all special cases and document why they aren't handled.
This would deal with #57 and #98.

I think we'd have to evaluate this on a case-by-case basis for distributions but I agree I think at least for the Gamma distribution it does not necessarily make sense to allow INF for α or β. I'll create an issue specifically for the Gamma distribution

FYI, we've recently had someone interested in supporting a particular degenerate case of Exp.

The use case referenced to justify degenerate parameter values is likely erroneous, an absorbing state can be modelled by having all outgoing paths loop back to the same state. In that case any distribution would work, unless one is interested in statistics about how many times a transition, including transitions to the same state, occurred.
I disagree with the proposed solution and agree with the Wikipedia page that the case is degenerate for a reason.
To this argument, mathematical infinity is more often than not not an actual element of the probability space but a statement of what happens when a limit is taken. If a distribution is dependent on a parameter α then the function can be seen as depending on two variables p(α,x). If any IEEE infinities or other degenerate cases are to be allowed, then one would have to either

treat all possible permutations of taking the limit and find a way to query the library for each of them, as they're unequal in general
agree to take the limit in one specific order
some mixture of 1. and 2.

Additionally to these complications, as stated above, either cdf can exist but pdf be a non-classical function as is the case with a discrete probability or both limits exist but aren't cdf's and pdf's anymore. If degenerate cases are allowed where not all the functions exist in a classical sense there's an argument to be made that returning errors is more helpful than approximating a delta function with p(x0) = infinity and for all other x   p(x) = 0

Perhaps I got lost in the mathematics on this one, I take some of that back. If there's only one parameter apart from the variable of interest then I agree, More general Exp is a reasonable extension. In the case of the gamma function however there are two parameters, so knowing in which order to take the limit doesn't have an equally reasonable default.

Thanks for the analysis and follow up comment. I agree: the exp case seems unambiguous, but in general that is not the case. It may be that allowing for degenerates in only one variable is also a sensible option (but I haven't thought enough about the gamma case).

Author of the More general Exp issue here.

What also simplifies the resolution in the rand crate is that there no evaluation of pdf is sought, only a consistent extension of sampling.

Exactly!! This is what I had in mind. In the case of Exp, it would be an expected behaviour to sample infinity.
In the other hand, a Gamma with both parameters infinity has no intuitive behaviour.
Having said that, one should either document the chosen behaviour (as it has been done in statrs) or not support it to start with.

I think we'd have to evaluate this on a case-by-case basis for distributions

I totally agree with that, but not only because there can be an intuitive answer for some cases but also because there can be useful (and arbitrary) implementations. Just as an example: assert_eq!(std::f64::INFINITY, 1.0 / 0.0) this behaviour simplified our code, but there is no correct answer to what 1 / 0 is.

Just as an example: assert_eq!(std::f64::INFINITY, 1.0 / 0.0) this behaviour simplified our code, but there is no correct answer to what 1 / 0 is.

As I outlined with the mathematical infinities that get confused with IEEE754 infinities, the semantics are not identical. This starts to get unintuitive because -0.0 == 0.0 which respects the mathematics but 1.0 / -0.0 != 1.0 / 0.0, so in some cases the floating point numbers are supposed to be interpreted as limits while in other contexts this isn't the case. So yes 1 / 0 does not have a sensible solution as a number satisfying 0 * a = 1, but that's not the angle to take. Floating point expressions can either be made sensible as numbers, made sensible as limits or neither of those, which leads to the at first unintuitive property of NaNs not being equal to themselves. That's akin to the situation with functions here.

Yes, but you missed the bit about convergence: 1/x → +inf as x → 0 from above. "INF" is not a real mathematical value, but a model of what you observe when approaching a degenerate value.

Based on the discussion my current proposal would be

Allow singly degenerate parameter values, limit taken first over parameter, then over argument (x in the above thread).
Future-proofing for multidimensional arguments by also only allowing singly degenerate argument evaluations
Return NaN if any function limits don't exist as classical functions.

I think an error is preferable to NaN on the last point, but this is at least not immediately desirable due to to being backwards-incompatible.

I agree with these principles and would add the following:

If a multi-parameter case has defined behaviour, there should not be a problem in adding it.

I am not sure the consequences of following these premises, but could start with something:

Gamma distribution should not accept both parameters (alpha and beta) as infinity. But, it should allow one of them to be infinity.
Normal distribution should not accept mean infinity and std_dev infinity at the same time. But, should allow: mean infinity and std_dev finite (returning infinity always); mean finite and zero std_dev (returning a constant equal to mean always); mean finite and infinity std_dev should not be allowed, as it does not converge in a reasonable way.
...

Of course, the list can be longer and implementing all these changes might not be what we want(?)
The possibility is to implement them "as it is requested".

If a multi-parameter case has defined behaviour, there should not be a problem in adding it.

I agree, caveats below, especially since errors are returned when constructing the distributions, it's an immediate feedback on what to expect.

Normal distribution should not accept mean infinity and std_dev infinity at the same time. But, should allow: mean infinity and std_dev finite (returning infinity always); mean finite and zero std_dev (returning a constant equal to mean always); mean finite and infinity std_dev should not be allowed, as it does not converge in a reasonable way.

This is at odds with the rand crate. What you're saying here is that all statistical properties are derived from the distribution of the random variable, so if no limit of the distribution exists we should give up and not construct the distribution, disabling any properties we could call from it. More general Exp however just cares whether the properties derived have a consistent extension. For example, using the logic used to extend the exponential clock we could conclude that the sampling of a normal distribution with mean 0 and infinite variance is infinity with probability 1/2 and -infinity with probability 1/2.

The possibility is to implement them "as it is requested".
Strongly disagree. We should model the established mathematical practices as closely as possible. Convergence of random variables have formal methodologies for characterising random variables are degenerate cases of each other. I thus reverse my reversal on the issue and think more general Exp is a bad idea due to selectively extending properties derived from a distribution that cannot formally be shown to converge against any distribution. It breaks mathematics in subtle ways. The principled approach would be to pick one of the convergence metrics and extend to degenerate cases if such a limit in the chosen metric exists, and deny deriving any properties from inexistent distribution limits.

I agree, caveats below, especially since errors are returned when constructing the distributions, it's an immediate feedback on what to expect.

Yes, indeed! and any function that can fail should return a Result, that is the idea :)

Finite mean and infinity std_dev do not make the distribution converge

Exactly, that is why I proposed to not accept this case either:

mean finite and infinity std_dev should not be allowed, as it does not converge in a reasonable way.

Convergence

More general Exp does not care whether the distribution converges, only if one selected property can be extended consistently.

Maybe it was not so explicit, but the convergence we considered to solve More general Exp was "the intuitive behaviour a user would expect", which I think is still the correct thing to do.
To translate into a mathematical convergence one needs a sequence of random variables. Sadly, there is no "good sequence" to consider. Let's say we want to claim something like "Exp(0) is the almost sure limit of a sequence of Exp(lambda) when lambda goes to zero". One can prove that the almost sure convergence depends on the sequence of lambdas we consider. Therefore, there is no way we can put a criteria like "let's accept the limit when it corresponds to the almost sure convergence". (see Appendix)
Intuitive convergence
If intuitive convergence is not almost sure convergence, what should it be?
As you point out, the convergence of distributions is not a "good" convergence either, since More general Exp would not be right in this case.
I think that, if there is a characterization of Exp(lambda) = Exp(1) / lambda, then it is easy to agree in the limit when lambda goes to zero. Basing the limit cases in characterizations like this is, of course, relative to the particular characterization and the limit might "break" under other characterizations.
Usually, such a characterization is the one used when simulating the variable, so in the context of the rand crate is easy to get an agreement. Maybe, if such a characterization is also part of the documentation in statrs, then the expected behaviour will also be "intuitive".
Practicality

The possibility is to implement them "as it is requested".

Strongly disagree. We should model the established mathematical practices as closely as possible.

This is purely from a practical point of view. Of course one would like the crate to be as complete as possible.
Appendix
Consider a sequence of variables X_n with distribution Exp(lambda_n). We will show that, depending on the sequence (lambda_n)_{n > 0}, we can have that (X_n) converges almost surely to infinity or not.
Proof:
Case 1: not convergent.
First, consider a decreasing sequence (alpha_n) that goes to zero. The idea is to take increasingly more copies of alpha_n so that we can ensure that there is no convergence to infinity. Consider alpha_1, then, take m_1 copies of an Exp(alpha_1) so that the probability that the minimum among these variables is less than one is greater than (0.9)^{1 / 2^n}. Then go to alpha_2 and do the same to define a new number m_2, and so on for each alpha_n define m_n.
Consider the sequence (lambda_n) as the decreasing sequence of all alphas considered in the previous step, with the number of repetitions considered. The idea is that the sequence (Exp(lambda_n)) has so many chances to be below one, that the liminf of the sequence is not infinity and therefore its limit either. Indeed, consider n such that lambda_{n+1} < lambda_n, i.e. just after "changing to a lower alpha". Then, the probability that in the last m_n variables the minimum is less than one is at least (0.9)^{1 / 2^n}. Therefore, the liminf is less than one with probability one.
Case 2: convergent.
Consider a sequence of Exponential variables X_n with distribution Exp(1 / n^2).
We need to prove that the probability of lim_{n \to \infty} X_n = \infty is one.
Equivalently, we can prove that, for all K > 0, we have that the probability of liminf_{n \to \infty} X_n > K is one.
Define the event E_n = {w : X_n(w) <= K}. Note that (limsup E_n)^c is the event liminf X_n > K. Let us prove that the probability of limsup E_n is zero. The probability of E_n is 1 - e^{-K / n^2}, which is less than K / n^2 (linear expansion of exponential around zero). Using Borel-Cantelli's lemma, since the sum over the probability of E_n is finite, we have that the probability of limsup E_n is zero, which means that the probability of liminf X_n > K is one. Since K is arbitrary, we conclude that the liminf of (X_n) is infinity and therefore the sequence converges to infinity almost surely.

the limit of sampling of a normal distribution with mean 0 and infinite variance is infinity with probability 1/2 and -infinity with probability 1/2, so does have a limit

This is still divergence, so I don't think it's valid to say that it has a limit.

The possibility is to implement them "as it is requested".

In practice, many things don't get implemented until requested. But if we can tackle degenerate cases comprehensively, that would be great. (This also implies documenting why certain degenerate cases should not return a solution.)

Let's say we want to claim something like "Exp(0) is the almost sure limit of a sequence of Exp(lambda) when lambda goes to zero". One can prove that the almost sure convergence depends on the sequence of lambdas we consider.

Example? The distribution is only (originally) defined for on positive parameters. Ah, this is "Case 1: not convergent." This is an (informal) proof that for any λ > 0, there is some ε > 0 such that for X in E(λ), P(X < 1) > ε. This is a very weak statement, especially considering the inherent limitations of computing (maximum number of samples, limits on precision of common data types).
It should be clear that for any fixed ε > 0 and k > 0, there exists some λ > 0 such that for X in E(λ), P(X < k) < ε. Is this not enough to say lim{λ→0}( X ) > N where N is our largest finite representable number?

To translate into a mathematical convergence one needs a sequence of random variables. Sadly, there is no "good sequence" to consider. Let's say we want to claim something like "Exp(0) is the almost sure limit of a sequence of Exp(lambda) when lambda goes to zero". One can prove that the almost sure convergence depends on the sequence of lambdas we consider. Therefore, there is no way we can put a criteria like "let's accept the limit when it corresponds to the almost sure convergence". (see Appendix)

Yes, that was exactly what I was trying to say. It does not converge almost surely, which is why there's a mismatch between rand and statrs. It seems to me we can agree that the returning of errors upon calling T::new() for degenerate cases means there is some notion of convergence, ideally I would think a formal one, for which such a limit does not exist. This prevents statrs from calling sample on such a function, as it returns an error for Exponential::new(0.0).
Nit-pick: When a parameter is real the definition of convergence is it converges no matter what discrete parameter sequence converging towards the degenerate value is chosen.
It boils down to two questions:

Should statrs and rand be consistent?
Should one care about lim z→z' H(pdfz) or lim z→z' pdfz for degenerate z' values, where H is a function dependent on pdf?

@dhardy

This is still divergence, so I don't think it's valid to say that it has a limit.

I'm aware this is not a formal argument. I was using the intuitive version of convergence to sample how far one is willing to take this.

This is a very weak statement, especially considering the inherent limitations of computing (maximum number of samples, limits on precision of common data types).

I'm not sure where this is heading. Are you suggesting to implement different library functions for f32,f64...?

Are you suggesting to implement different library functions for f32,f64...?

Actually, yes, but not now. It's also besides the point. The point is that for any situation using one of these libs, there is some limit on precision and some limit on the number of samples. It doesn't matter what those limits are, only that they exist; given that, the argument above is that for Exp(0), all observable samples should be +INF.
I haven't really considered other distributions, yet.
For Norm(μ, σ²), one gets into problems once σ² gets close to the limits of precision. Since a sample is generated as self.mean + self.std_dev * n, FP arithmetic may overflow to ±INF even when all parameters are finite. To prevent that we'd need some arbitrary bound on std_dev much smaller than INF. Note that the situation for norm = 0, std_dev = INF is not that different than for std_dev finite but close to INF: both may generate +INF and -INF values (although the latter case can also generate finite values). Whether or not std_dev = INF is allowed, the user has to be able to handle INF outputs if parameters may approach the limits of FP range. And NaN can be output if (1) mean and std_dev are both non-finite or (2) if std_dev is non-finite and the standard-normal sample n = 0. There is also an issue that if mean and std_dev are both close to the maximum finite value, then -INF values may be generated almost as often as +INF values (since when n × std_dev evaluates to -INF, adding a large but finite mean value does nothing).
The above analysis is to say that however we choose to solve this problem, (a) case-by-case analysis will be needed and (b) one cannot consider this a pure-maths problem.
Perhaps we should start by asking questions like:

Is it reasonable to generate ±INF values? As the above shows, out-right preventing this from happening is probably impossible. As @rasa200 was arguing earlier, in practical terms, this can be useful.
Is it reasonable to generate NaN values? In my opinion we should try to prevent this, since (a) such cases surpass the mathematical limits of the FP format and possibly also of real numbers, thus reasonable analysis is no longer possible and (b) NaN values corrupt all FP arithmetic (preventing any useful results), and even worse, operations like x < K evaluate to false whenever x is non-finite since booleans do not have an error value. Note however that completely preventing NaN may not always be possible or reasonable (although I think we already agreed that in the case of the Normal distribution, variance must be finite).

When a parameter is real the definition of convergence is it converges no matter what discrete parameter sequence converging towards the degenerate value is chosen.

The issue with this approach is that we are not dealing with real numbers, we are dealing with floating points (or finite precision numbers), which include infinity. Therefore, one should not focus so much in the math here, and more in the actual numbers on the computer.
Note: Even with arbitrary precision, the source of randomness needs to have a finite precision, so there is no such thing as arbitrary precision while simulating continuous random variables.
INF is useful, for example, to convey: "precision limit overflow". Returning NaN would express too little of what is actually happening and, in some situations, +INF or -INF can be used by the user.

(a) case-by-case analysis will be needed and (b) one cannot consider this a pure-maths problem.

Totally agree.

Is it reasonable to generate ±INF values?

Well, you know my answer. It boils down to "it can be more useful to an user than an error", especially since INF can be already obtained (recall finite precision), as @dhardy pointed out.

Is it reasonable to generate NaN values?

I think it would be strange for a user to get a NaN value.
I do not know of any "reasonable" case in which it is the right answer, but there might be.
If I get NaN while doing computations, it is because either (1) I went beyond the precision, or (2) some computation can actually fail and I should replace it with another.
For me, NaNs are like errors, and we should try to document them if there is a possibility of returning them.

Since a sample is generated as self.mean + self.std_dev * n, FP arithmetic may overflow to ±INF even when all parameters are finite. To prevent that we'd need some arbitrary bound on std_dev much smaller than INF.
[...] There is also an issue that if mean and std_dev are both close to the maximum finite value, then -INF values may be generated almost as often as +INF values (since when n × std_dev evaluates to -INF, adding a large but finite mean value does nothing).
[...] one cannot consider this a pure-maths problem

If evaluating an expression in infinite precision and range would yield a value that should not overflow/NaN when the result is expressed in fp, for example (f64::MAX + 1.0) - 1.0, yet the fp calculation yields overflow/NaN, then this is due to intermediate rounding/overflow errors propagating and it is the job of numerical analysis to find an expression that can be accurately evaluated in fp. There's no disagreement in that.
If finding different expressions that overflow less for the given range is your goal, like (mean + std_dev / 2.0 * n) + std_dev / 2.0 * n where mean is close to -inf, std_dev is close to inf and n is positive, I would suggest filing a separate issue, as this conversation is getting difficult to follow.
OP was about what to do with point-blank degenerate values and to figure out what the sensible result should be, given the expressed interest in expanding the domain of the methods defined in statrs. Established practice in this library is to take the limit whenever mathematical functions have to be evaluated at degenerate values, which is why every pdf evaluates to 0 at inf. Otherwise not taking mathematical limits would lead to NaN values, like with the gamma function: NaN = inf * 0 = C * inf^(alpha - 1) * exp(-beta * inf). I was under the impression that this was not controversial, and thus suggested how to extend T::new() to degenerate values for distributions T. Since the object constructed by T::new() is a probability measure, I suggested the extension of T::new() to degenerate values be the analogue of the limit on real numbers, which is a limit in functions.
My proposed solution to treat point-blank degenerate values which have no issues of fp precision is to treat them as limits, which has the following properties

it's mathematically consistent
it has a precedent in this library
it agrees with fp evaluations for simple functions (1/inf = 0)
it's sometimes the only way to avoid NaNs when extending the domain (x * ln(x) at x = 0)

This conversation has switched rather abruptly from "what to do with degenerate inputs" to "what inputs to consider degenerate" and "what are reasonable outputs".
Since no effort was made to answer my questions about whether rand and statrs should be consistent and what limit to care about, I cannot say whether this is accidental.
In any case, fp precision doesn't change what to do with degenerate inputs, only what inputs to consider degenerate. Taking into account the fp precision means any checks x == INFINITY should be changed to x > f64::MAX and any x == a for normal a changed to (x - a).abs() < f64::EPSILON.
My answers to your questions are

Some reasonable effort should be made to avoid inf as an output if possible, by finding a new expression that does not overflow but accurately represents the mathematical function that is to be approximated. At some point you reach diminishing returns.
NaN should be avoided wherever possible. In general, you can't avoid using limits and NaN as output at the same time while extending the domain of a function as much as possible, like extending x * ln(x) from x in [f64::EPSILON,a] to [0,a] with some a such that there's no overflow.

Questions of my own are

whether you think statrs and rand should be consistent
what you think Exponential(0.0).unwrap().pdf(4.0) should be

@rasa200

The issue with this approach is that we are not dealing with real numbers, we are dealing with floating points (or finite precision numbers), which include infinity. Therefore, one should not focus so much in the math here, and more in the actual numbers on the computer.

I'm afraid I don't understand what you're saying. If I give you exactly inf as an input and ask you to make sense of it, something I cautioned in OP against doing, you've left the place where you have a reasonable expectation of fp giving you anything sensible. Occasionally this might happen, like 1/inf = 0, but often you'll get expressions like inf / inf, inf - inf, 0 * inf, where the crude limit-taking rules in fp will not give an answer universally true in mathematics, so it gives NaN. If you evaluate ln(x) / x for any sufficiently large x that doesn't overflow, it will be a roughly decreasing function modulo some fp rounding perturbations, until you hit a value such that x overflows and the expression gives NaN.
If the limit is not taken, what do you expect the pdf of gamma to yield at inf? It currently gives 0, a perfectly sensible and reasonable answer, motivated by pure math. If you're refusing to accept the answer provided by pure math after you've opted to go beyond what fp is reasonably capable of, yet want some value that's not NaN, I don't know what you're looking for.

If finding different expressions that overflow less for the given range is your goal

It's not my goal — I was merely pointing out other limitations, but as you say, it's a bit off topic.

My proposed solution to treat point-blank degenerate values which have no issues of fp precision is to treat them as limits

Where a mathematical limit exists, this is fine. The problem that came up was @rasa200's "proof of non-convergence" of Exp(0). Whether this proof is valid depends on the definition of convergence. Going back to these definitions... okay, we should skip to your question about PDFs.

what you think Exponential(0.0).unwrap().pdf(4.0) should be

It should be clear that lim{x→0} Exp(x).PDF is 0 everywhere. In this case the CDF is also zero everywhere, and hence continuous. The inverse CDF cannot be defined, but other than this I believe there is no issue.
Of course, one has to be careful; e.g. Wikipedia gives an example with discontinuous CDF which may be more of an issue.
This thus goes back to the question of which definition of convergence should be used?

whether you think statrs and rand should be consistent

Good question. I don't know. Rand does not implement CDF or PDF and probably won't ever do so, and tends to consider speed quite important (though we still haven't resolved whether to include high-precision alternatives). Even so, I don't believe there is a reason to choose different approaches to degenerate cases. @vks may also have an opinion.

Some reasonable effort should be made to avoid inf as an output if possible, by finding a new expression that does not overflow but accurately represents the mathematical function that is to be approximated. At some point you reach diminishing returns.

Usually only inputs close to FP limits can cause this. I don't believe we should care very much about this case, at least not when the only error is to generate obvious error/degenerate values like NaN and INF.

any checks x == INFINITY should be changed to x > f64::MAX

I believe the two are equivalent (there are only two infinity values).

any x == a for normal a changed to (x - a).abs() < f64::EPSILON

That's wrong (doc). In any case, one does not usually check direct equality, except at degenerate values, where this is not an issue.

It should be clear that lim{x→0} Exp(x).PDF is 0 everywhere. In this case the CDF is also zero everywhere, and hence continuous. The inverse CDF cannot be defined, but other than this I believe there is no issue.

You don't think it's awkward to allow returning a pdf without signaling an error, even though this mechanically extended pdf does not have the properties of a pdf, like integrating to 1?
What do you think should Gamma(1,inf).max() be? The limits do not commute. Taken as a limit in the almost-sure sense (limz->inf Gamma(1,inf)).max() = 0, while limz->inf (Gamma(1,inf).max()) = inf. The generalisation of exp in the rand crate deals with the latter limit. I argue the former limit should be used in statrs.
I think the edge case needed by @rasa200 can be dealt with by providing a Delta distribution that returns a constant when sampled, and making the constant inf.
If the limit of the distribution does not exist in the a.s. sense, statrs should return errors.

any checks x == INFINITY should be changed to x > f64::MAX

I believe the two are equivalent (there are only two infinity values).

There are some non-obvious things going on with IEEE754 which make me uncertain whether this is the case [0]. I think they're not equivalent.
Doing the latter comparison I believe protects against these oddities, hopefully.

any x == a for normal a changed to (x - a).abs() < f64::EPSILON

That's wrong (doc).

Thanks for pointing that out. Absolute difference is of course rubbish.
Approx seems to be an implementation of what is needed.
[0] https://arxiv.org/abs/cs/0701192

You don't think it's awkward to allow returning a pdf without signaling an error

I guess @boxtown should answer that since he's the maintainer of this lib. Yes, it is unexpected that the PDF integrates to 0 and the CDF is never 1, on the other hand it is still possible to implement the API according to its expectations (aside from those implicit ones on the properties of a PDF/CDF).
I would never expect numerical methods to be about to accurately quantify the integral of a PDF or inverse a CDF (without also providing bounds on the region of interest, which here obviously don't exist), hence this may not be a problem in practice. I don't know whether this is too awkward.

I'd like to get the degenerate cases removed and documented.
I hadn't noticed this discussion, but I've got an open PR for ensuring that Uniform is constructed on a bounded interval for this aspect of, "it is not easily clear what the behavior should be" to a user, so we will do our best to not answer such questions as a user can choose what works for them. I also extended the StatsError to have an enum for rejection based on finiteness.

Want to get the release out in hopes to get more feedback on what users expect on these cases.

@FreezyLemon would you say that you removed the apparent degenerate cases when replacing the errors?

@YeungOnion If you mean #265, I tried not to touch the existing behavior (what params get accepted/rejected), but instead made the error types more specific. #299 is IMHO a case of me explicitly removing a degenerate case from a distribution, but I haven't gone through all of them.


Make progress bar steps clickable

This is low priority, but it would be nice to be able to hop around without just forward and back.

progress is also not as apparent as it could be

Vertical might also be better use of space and more mobile friendly


"Save" button in modals is never re-enabled in case of failure

How are you running Flagsmith

[ ] Self Hosted with Docker
[ ] Self Hosted with Kubernetes
[X] SaaS at flagsmith.com
[ ] Some other way (add details in description below)

Describe the bug
When saving changes within a modal (i.e. feature states, segments, segment overrides, possibly others), the Save button is disabled while requests are still pending. However, if a request fails, the button is never re-enabled and requires a page reload to be clickable again. This effectively means losing all your UI state, which can be very frustrating.
Steps To Reproduce
See video below.

Force a request initiated by saving within a modal to fail somehow. For example, manually block a request needed to save a feature using developer tools:

Save a feature, forcing a request to be blocked
The Save button is never re-enabled and requires a full page reload to work again

Expected behavior
Saving within modals should have proper error handling and eventually always re-enable the Save button.
The error message should be displayed to the user, requiring manual dismissing to allow time to copy/paste or screenshot the error.
Screenshots
https://github.com/Flagsmith/flagsmith/assets/829698/a352a287-ed4f-4ac7-a989-bef9122a6fa6

Just to clarify, I think you're just meaning within the feature modal?

@kyle-ssg Yes that's correct, I thought I had reproduced it elsewhere but it's only in the feature modal.


Need to generate SDKs with both LLCs and convenience wrappers

For QnA Maker - though I suspect this may become more common with the push to LLCs - we need to generate a single SDK with LLCs for the authoring side of the APIs (conveniently in another file, if that helps) but convenience wrappers for the inference side of the APIs along with related models.
This currently does not seem possible, nor does the batch directive for autorest seem to work completely.

Duplicate of https://github.com/Azure/autorest.csharp/issues/1137


Idea : migrate all validations to ActiveModel

Instead of using its own rules, rails_param could use a ActiveModel class to validate the params.
This would be a major rewrite of the library.
The benefit is that the developer will be able to use all the known validations he already uses, like:
param! :phone_number, String, presence: true, length: { minimum: 3 }

Typecasting and transformation will still be implemented internally (I could use some other gems like Virtus but for the moment I don't want dependencies).
It will also be possible for the developper to access the errors Hash.

I liked this one, but seems you've been ignored. Maybe a pull-request could help?

A bit related — it can be done like this with Mongoid:
Search.fields.map { |name, field| "param! :#{field.name}, #{field.type}" }

Output
["param! :_id, BSON::ObjectId",
 "param! :created_at, Time",
 "param! :updated_at, Time",
 "param! :engine, String",
 "param! :q, String",
 "param! :output, String",
 "param! :location_requested, String",
 "param! :lat, String",
 "param! :lon, String",
 "param! :device, String",
 "param! :source, String",
 "param! :requester_ip, String",
 "param! :api_request, String",
 "param! :md5, String",
 "param! :proxy_provider, String",
 "param! :google_domain, String",

(It's a string array to show the code would be executed.)


What should I do if I want to build it by myself

I am really interested in how you implement the function of this plugin and if possible, I would love to provide some help in the plugin if possible.
However, I am unable to build the plugin by maven (I am somehow new to maven).

https://www.youtube.com/watch?v=6AVC3X88z6E
You can watch that video ^^

Thanks! I will take a look on that! 😉😉😉😉😉


Lazy enlisting

There's a problem in specs ...
I wouldn't be surprised if you stopped reading after this point, since it's usually not a worthwhile justification.
But please take a moment and bear with me.
If we define a cop for testing purposes like that:
module Dept
  class MyCop < RuboCop::Cop::Cop
    # ...
  end
end

you'll end up with a constant littering the root namespace.
https://rspec.rubystyle.guide/#declare-constants
https://fili.pp.ru/leaky-constants.html
It's not a big deal if you pick unique class and namespace each time or are brave enough to deal with potential side effects.
The following is arguably better:
klass =
  Class.new(RuboCop::Cop::Cop) do
    def external_dependency_checksum
      'something other than nil'
    end
  end
stub_const('Dept::MyCop', klass)

But there's a problem with this approach:
  1) RuboCop::Cop::Team#external_dependency_checksum when cop with different checksum joins has a different checksum for the whole team
     Failure/Error: new(*class_name.split('::').last(2))

     NoMethodError:
       undefined method `split' for nil:NilClass
     # ./lib/rubocop/cop/badge.rb:26:in `for'
     # ./lib/rubocop/cop/cop.rb:66:in `badge'
     # ./lib/rubocop/cop/registry.rb:35:in `enlist'
     # ./lib/rubocop/cop/cop.rb:62:in `inherited'
     # ./spec/rubocop/cop/team_spec.rb:372:in `initialize'
     # ./spec/rubocop/cop/team_spec.rb:372:in `new'
     # ./spec/rubocop/cop/team_spec.rb:372:in `block (4 levels) in <top (required)>'

Backtrace explains it all, but in a nutshell there's a pesky def self.included in Cop that enlists the newly defined cop for a department. And to get the department, it needs a name. Which is nil for Class.new.
Ha! Easy-peasy you think: define a class method name on that cop.
Not so easy it turns out:
class A
  def self.inherited(base)
    fail 'Ooopsie!' unless base.name
  end
end

class B < A # ok
end

Class.new do # boom!
  def self.name
    'Multipassport!'
  end
end

And guess what? self.inherited is called before the block is evaluated.
Class.new to blame you would say!
But it's not. Here's another example for you.
class A
  def self.inherited(base)
    fail 'Traitor!' unless base.surname
  end
end

class B < A
  def self.surname
    'Schtirlitz'
  end
end

NoMethodError: undefined method `surname' for B:Class

Srsly? WTH, Ruby? They have to quack, but some of them are real and have to have names and other things!
Ok, if it's not Class.new to blame, it's a premature inherited hook. What do we do?
Lazy enlist to the rescue!
This started off as https://github.com/rubocop-hq/rubocop/pull/8018#discussion_r429582159

Before submitting the PR make sure the following are checked:

[x] Wrote good commit messages.
[-] Commit message starts with [Fix #issue-number] (if the related issue exists).
[x] Feature branch is up-to-date with master (if not - rebase it).
[x] Squashed related commits together.
[x] Added tests.
[ ] Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
[x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
[ ] Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

This innocent seemingly self-contained change knocks down specs all around RuboCop.
I would kindly appreciate any help and guidance on how to fix them and will address any cosmetic review notes. However, due to different priorities, I'm not up for a deeper investigation or multi-class refactoring at this moment.
ruby-x.y-rubocop build job failures seem unrelated to my changes.

I was wondering why putting those cops in the 'Test' department isn't good enough? Note that Cop::Cop.all excludes those.

I could have said that some extensions had to use ugly crutches to work this self.inherited around, but it seems that it's not completely true.
There's still this though:
class RuboCop::Cop::RSpec::Cop
  ...
  def self.inherited(subclass)
    RuboCop::Cop::Cop.inherited(subclass)
  end
end

for a reason that will become clear from this example:
class BaseBase
  @registry = []
  class << self
    attr_reader :registry
  end

  def self.inherited(base)
    registry << base
  end
end

class BaseExt < BaseBase

end

class Cop < BaseExt
end

# NoMethodError: undefined method `<<' for nil:NilClass
# from (pry):8:in `inherited'

BaseBase.registry # => [BaseExt]

class BaseExt
  def self.inherited(base)
    superclass.inherited(base)
  end
end

class AnotherCop < BaseExt
end

BaseBase.registry # => [BaseExt, AnotherCop]

I'm proposing to move the registry in #7868, one reason being that it should not be stored in an attribute of a class that people inherit from... I believe it would resolve this issue.

Sure it will 👍 I like the proposed change to autocorrect syntax as well.
However, this specific change is only to improve specs (also across extensions) and doesn't contradict with your larger-scale refactoring.

For specs, in my PR there's an easy way:
around do |test|
  described_class.with_temporary_global { test.run }
end

This runs the tests while reseting the global registry afterwards. Couldn't think of a better name though 😅
One thing I like about the idea of lazy enlisting is that it would allow an easy way to have a not_enlistable or similar method that could remove a cop class from the list, for easy abstract base classes.
I would favor a slightly different implementation though, so that it's possible to enlist cops even after calling cops or something. If I'm not mistaken, after call_for_serve_and_protect! enlisting wouldn't work at all.

Haven't tested it, but something like https://github.com/marcandre/rubocop/commit/0bb18cb22ff918ccd9599301fc5df4768b0e29b3

Merged #8095, closing this.

Nice!
But did #8095 solve the issue with leaky constants in specs? https://github.com/rubocop-hq/rubocop/pull/8023/files#diff-23dfc9af273ca5d2af6c37e1aef3c0c5L10

I still see
      NoMethodError:
        undefined method `split' for nil:NilClass

if I:
diff --git a/spec/rubocop/cop/registry_spec.rb b/spec/rubocop/cop/registry_spec.rb
index 0524230fa..0cdaf5678 100644
--- a/spec/rubocop/cop/registry_spec.rb
+++ b/spec/rubocop/cop/registry_spec.rb
@@ -4,27 +4,6 @@ RSpec.describe RuboCop::Cop::Registry do
   subject(:registry) { described_class.new(cops, options) }

   let(:cops) do
-    stub_const('RuboCop::Cop::Test', Module.new)
-    stub_const('RuboCop::Cop::RSpec', Module.new)
-
-    # rubocop:disable RSpec/LeakyConstantDeclaration
-    module RuboCop
-      module Cop
-        module Test
-          # Create another cop with a different namespace
-          class FirstArrayElementIndentation < Cop
-          end
-        end
-
-        module RSpec
-          # Define a dummy rspec cop which has special namespace inflection
-          class Foo < Cop
-          end
-        end
-      end
-    end
-    # rubocop:enable RSpec/LeakyConstantDeclaration
-
     [
       RuboCop::Cop::Lint::BooleanSymbol,
       RuboCop::Cop::Lint::DuplicateMethods,
@@ -37,6 +16,11 @@ RSpec.describe RuboCop::Cop::Registry do

   let(:options) { {} }

+  before do
+    stub_const('RuboCop::Cop::Test::FirstArrayElementIndentation', Class.new(RuboCop::Cop::Cop))
+    stub_const('RuboCop::Cop::RSpec::Foo', Class.new(RuboCop::Cop::Cop))
+  end
+
   # `RuboCop::Cop::Cop` mutates its `registry` when inherited from.
   # This can introduce nondeterministic failures in other parts of the
   # specs if this mutation occurs before code that depends on this global cop

Please disregard my previous comment, I forgot to update master.
All cool. I'll open a PR to clean up specs for which #8095 allows now 👍

Right, it doesn't remove the leaky constants, but it makes it easy to not affect the global Registry.
And yeah, that error of a Cop class not having a name just yet is solved with the lazy enlisting.


Product with custom option quantity increase

Current behavior
When I add the same product but with different custom options the quantity are increasing automatically.
One simple product added with custom options

Second simple product added with custom options (first one has increased and should not)

On page refresh the first one quantity increase again and each time I refresh

Expected behavior
The product quantity should not increase why I add the same product with different custom options and on refresh.
Steps to reproduce the issue

Create Magento 2 shop
Create a product with custom options
Go on VSF page and select multiple custom options and 1 qty
Add the product to the cart
Change the selected custom options
Add the product to the cart (first product quantity has increased to 2)
Refresh the page (first product quantity has increased to 3)

Version of Vue Storefront

[x] Vue Storefront
[ ] Vue Storefront Next

Can you handle fixing this bug by yourself?

[ ] YES
[x] NO

Which Release Cycle state this refers to? Info for developer. (doesn't apply to Next)
Pick one option.

[ ] This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
[ ] This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
[x] This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

Browser: Version 83.0.4103.116 (Official build) (64 bits)
OS: Ubuntu 21
Node: v10.21.0
Code Version: v1.12.2

Additional information
Similar to this issue on a side note
"In addition I just realized that if you refresh the page the quantities of products with custom options are doubled while the quantities of the other products are not affected."
https://github.com/DivanteLtd/vue-storefront/issues/3976

Do you've some news on the progresss of this ticket ? Thanks in advance


Manage rule not applying to drop down list

This seems related to #676. For fantastical, when I open the helper from the menubar, select an event and click a drop down (e.g. to choose an alert time), the drop down popup appears briefly and is immediately closed.
I've got this rule which I know works for the main helper window:
yabai -m rule --add app="^Fantastical Helper$" manage=off border=off

And I know it's an issue with yabai because the problem goes away when I stop the service.

It seems to be related to window_topmost on as the issue goes away if that's off.

Heads up, if you want to be able to still keep floating windows on top of other applications being used, but not have to deal with the issues that come with using topmost=on you can create an array of applications that you don't want to have covering floating windows and then set their layer to below. For example, something like:
# DEFAULTS
yabai -m config layout bsp
yabai -m config window_topmost off

# APP SETTINGS

# applications with custom rules (main, day-to-day applications/fullscreen applications that i want to stay behind floating windows)
applications="(Calendar|Mail|Reminders|Notes|Microsoft Outlook|Discord|Messages|Slack|Firefox|Chrome|Code|Microsoft Word|Preview|Terminal|iTerm2|QuickTime|Player|Spotify|Music)"

# any apps that aren't main applications get set to float
# e.g. system preferences, colour meter
yabai -m rule --add app!="^${applications}$"    manage=off border=off

# main applications get set to bottom layer to prevent them from
# covering floating applications/popup windows
yabai -m rule --add app="^${applications}$"     layer=below

BTW, I've noticed that layer and topmost settings seem to persist after being commented out -- so if you're testing anything to do with these you need to actually reset them to normal/off before commenting out/removing them, otherwise they'll just keep behaving as though nothing's changed (been tossing up creating a separate issue for this, but I'm not sure if it's worth it or not?).

Further example: Ignore settings window of LibreOffice (manage=off) and set layer=above with
yabai -m rule --add app="^LibreOffice$" title="^(Delete Contents|Customize|About LibreOffice|Check for Updates|Options - LibreOffice - .+)$" manage=off layer=above

The drop down menus in the setting windows are not accessible.

Should be resolved in latest version.


Hardcoded parameters on desktop

There are a few hardcoded parameters on the Electron clients:
https://github.com/Jigsaw-Code/outline-client/blob/e26267f914a3982f4e1518b41efefaa6ff41e444/src/electron/process_manager.ts#L30-L37
That can cause conflicts and possibly some of the fail to connect issues.
We should get them dynamically. At least the port number.

We do this in the manager by opening Node servers on random ports until we find a free one.  This introduces a slight race condition in between when we close the server and when we use the port we found.  We could get around that by either keeping the node server open until tun2socks starts up (if tun2socks doesn't return an error in that case) or to loop the find-a-port -> close-server -> open-tun2socks process until tun2socks starts successfully or we encounter a different error.
Maybe we can use whatever strategy we use for #954 and get both issues in one go

It seems the port is not an issue on the go-tun2socks branch, so I think that's covered. Then the priority is to finalize the migration of the desktop clients to the Go stack.
We may still want to consider the other parameters, but they may be less critical.

Awesome, I'll tentatively close this as obsolete and we can re-open if we need to.

It's not in master yet, so I'll reopen until it is.


STX transfers pending forever (14+ hours)

Helping a user on Telegram, usually in this case (forever pending STX transfer) the nonce is wrong or the balance is insufficient but this transaction has been pending for 14 hours and I do not understand why. Nonce is correct, and balance is sufficient.

https://explorer.stacks.co/address/SP39WQ90WHTTN6BTTW8GST8FJXPZVVX8R2JESYT44?chain=mainnet
The only weird thing about this list of transactions is the ones below with nonce 12-15 they appear to be pending but when you go into the transactions it directs me to transactions that are mined yet failed: https://github.com/hirosystems/explorer/issues/864
Why are miners not picking this up?

After the user increased the fee (2x) it did get picked up right away. https://explorer.stacks.co/txid/0x3b44d950da24dcbd7f0dca14b3764ce7064528e015594288d77933f2e205e970?chain=mainnet
The fee was already above average prior to the increase though, still strange it was not picked up before especially over such a long time.
Average fee lately has been around 0.063 according to: https://stacksonchain.com/dashboards/Transactions-per-Day/14

Can this be closed?


[docs] Fix casing typo

[x] I have followed (at least) the PR section of the contributing guide.

@piperchester Thanks


Proactively alert users, when they will be unable to proxy with a friend because of NAT configuration

This requires

using normal stun servers: #1111
probing the NAT configuration when uProxy is loaded (could be configurable, but defaulted to ON)
communicating NAT configuration over signaling channels: #1478
explaining non-functional NAT pairings to users: #1482

We'd also presumably want some UX for an error we can show next to a user, to indicate that proxying with that person will probably fail.

Handling incompatible NATs will always be a problem to some extent. Just let's not forget that on the backend side we do want to support TURN servers at some point, which would give us compatibility with many symmetric NATs:
https://github.com/uProxy/uproxy/issues/191

Closing as this is not a priority right now


Possible issue with BilinearForm::AddDomainIntegrator with marker

I ran into an issue using a mesh with non-contiguous domain attribute tags and think the culprit is this check in BilinearForm::Assemble:
 if (domain_integs_marker[k] != NULL)
 {
    MFEM_VERIFY(mesh->attributes.Size() ==
                domain_integs_marker[k]->Size(),
                "invalid element marker for domain integrator #"
                << k << ", counting from zero");
 }

I think the correct behavior would be to check mesh->attributes.Max().
The boundary integrators with marker have a correct check that works with non-contiguous attributes, see the code starting at this line.

I think you are right, there is an inconsistency in how domain_integs_marker[k] is used here (A): https://github.com/mfem/mfem/blob/167ff72edccc4e5b0a642ae3392ccaf50f455b9e/fem/bilinearform.cpp#L438-L439 and the check here (B): https://github.com/mfem/mfem/blob/167ff72edccc4e5b0a642ae3392ccaf50f455b9e/fem/bilinearform.cpp#L418-L421 and also the documentation here (C): https://github.com/mfem/mfem/blob/167ff72edccc4e5b0a642ae3392ccaf50f455b9e/fem/bilinearform.hpp#L94-L98
Basically, (B) and (C) are wrong and the size of domain_integs_marker[k] (when not NULL) should be mesh->attributes.Max() or 0 if mesh->attributes is empty.
There is a similar issue in LinearForm, here: https://github.com/mfem/mfem/blob/167ff72edccc4e5b0a642ae3392ccaf50f455b9e/fem/linearform.cpp#L123-L126 and in the documentation here: https://github.com/mfem/mfem/blob/167ff72edccc4e5b0a642ae3392ccaf50f455b9e/fem/linearform.hpp#L36-L40
Thank you for reporting this issue. Would you like to propose a PR to fix it?

Great, that's what I thought. Happy to go and make a PR, I should be able to do this next week. Thanks for checking those other spots!

Discussion continued in #2648


Add DynamoDB update code path

Currently the DynamoDB controller only support create and delete operations. We should add Update operation code path as well.
/assign

Since ACK treats GlobalTables and Tables as two different and independent resources, we still need to understand what are about the boundaries between these two resources from the DynamoDB Table/GlobalTable controller plane view, and what are/are there any overlapping calls that might impact two different resources (Table and GlobalTable).
Questions to answer/investigate:

How does the GlobalTable controller plane remove Replication group? are the deletion actions queued? Is it possible to interrupt the deletion by adding more replication groups?
Let's assume we request a deletion of all the replication groups. What happens if a user calls after a short amount of time UpdateGlobalTable to re-increase the GlobalTable ReplicationGroup? Will the deletion still happen or will be interrupted?
How does a “UpdateTable” call (used to update the replicas GlobalSecondaryIndexes ) impacts DescribeGlobalTable response?
And vice versa does a UpdateGlobalTable impacts the response of a DescribeTable ?

This issue also includes:

Enabling table stream https://github.com/aws-controllers-k8s/community/issues/1000 (part of (table update path)
Adding TImeToLive field https://github.com/aws-controllers-k8s/community/issues/911  (extra API calls during the update code path)

Fixed and released in dynamodb v1.1.0
/close


remove deprecation warning about service display being renamed

service display was renamed to visible to avoid a name collision with the Kernel#display method (added in https://github.com/ManageIQ/manageiq/pull/19211)
and the rename itself got merged in https://github.com/ManageIQ/manageiq-automation_engine/pull/449
so we no longer need the warning

@miq-bot add_label technical debt

@miq-bot assign @tinaafitz

@kbrock /review request please

Revisiting this, I'm not sure that we should have hidden this and removed the deprecation.
We need to have some guidelines and a process/roadmap as to when we can remove values from the api and product.

Revisiting this, I'm not sure that we should have hidden this and removed the deprecation.

That would indeed be precisely why I asked what was happening with the mixin.


Create total population row (served+unserved)

Total (with blanks across all rows) for the local authority, is not being calculated. The needs to be caluculated and presented in the final df and csv.

Have ran into this issue when processing Scotland data in #284
This needs to be implemented for both the England & Wales Script and for Scotland. The changes could happen within one of the data_transform module.

I'm going to start looking at this.

Have gone into debugging mode through the script.
I think it doesn't create this row because in reshape_for_output function in data_output when the melting occurs the variable total is both in the id_vars & value_vars arguments.

Manual examples of what this should look like can be found in #188

Have added in some extra code within the data_output function.
Here are examples outputs for both the Eng&Wal and then Scotland outputs
All_results.csv
Scotland_results.csv


Original URL is not preserved in oauth redirect URL.

Describe the bug
After user authentication using oauth2, the ambassador redirect URL does not preserved the original URL. Portion of the URL is truncated.
To Reproduce
Steps to reproduce the behavior:

Setup oauth2 filter
Invoke page with the url with passing additional parameter. Example here is : https://azdeploy.bdns.dev.in/MyApp1/app/#test/tcomp/infoPage?oid=VR%3Acom.change.ChangeRequest2%3A196141&&u8=1
3.Redirect to IdP for authentication.
After  authentication, check the ambassador URL redirect. The portion - "test/tcomp/infoPage?oid=VR%3Acom.change.ChangeRequest2%3A196141&&u8=1" is not in the redirect URL.

Expected behavior
Expected that redirect url should the have original page url
Versions (please complete the following information):

Ambassador: 1.13
Kubernetes environment: AKS
Version : 1.18

Additional context
Add any other context about the problem here.

Answered by @selenaanguyenn: We were able to replicate this issue which has to do with how URLs work. When a fragment ("#") is used in a URL, the part of the URL after the fragment, the fragment identifier, is not sent in the HTTP request message. This is because the fragment identifier is only used by the client or browser. Because the fragment identifier is never sent in the request, Ambassador and the OAuth 2.0 Filter never know the fragment identifier, which is why the Filter redirects to a truncated version of the original URL. This truncated version is the part of the URL before the fragment. This URL behavior should be what you are experiencing since you are seeing this pattern with all URLs with "#".
Because this is the nature of URL fragment identifiers, this is not something that is changeable by Ambassador. If you were to try a workaround such as injecting the fragment identifier into a header before sending the request, that would have to be done on the client or browser side. Instead of going through that, though, we would recommend that you adapt a workflow where the user has to be logged in before accessing any URLs with fragment identifiers or anchor links. This would prevent URL truncations after SSO and OAuth 2.0.


fail-if-empty not used in run/operation resolution

See https://my.guild.ai/t/self-requiring-operation/886
This should work:
op:
  requires:
    - operation: op
      fail-if-empty: no

This is bogus - fail-if-empty is working fine. The issue is that the OP needs an option dependency.


Explicitly list publishable workspaces

see https://github.com/rwjblue/release-it-yarn-workspaces/issues/47

Seems fine, but I'd vaguely prefer to just fix that bug instead...

yeah +1 for sure on fixing the bug


Should we use DNS or service discovery?

For resolving featuros backend (dashboard) we could use DNS or service discovery (zookeeper, etc.). Or we can do both.
Discuss!

For resolving featuros dashboard we could use DNS or service discovery (zookeeper, etc.) for resolving featuros dashboard? Or we can do both.
Discuss!

Do you mean 'how should featuros-plugin locate the back-end it reports to'?
I feel the need for a versions dashboard has its source - to some extent - in lack of zookeper integration for most of our apps. At least this was one of the things that made me think about such a dashboard.
I think that not being able to use the dashboard with apps not using zookeper would be a serious drawback. That's why I'd suggest first locating the dashboard via dns and later on resolving it using zookeeper.
Also, I think zookeeper could be of use itself: I think for a zookeeper-registered app we could get some of the information featuros-plugin intends to provide without even having the plugin in the app. Or, at least, we could detect apps that still need to be assimilated.

Thanks for the correction, I've fixed the description.


Replace useEffect by useLayoutEffect

This PR fixes #8
Thanks @sintylapse for such a great finding!

Thanks for a quick fix


Improve compatibility with application bundlers

This PR allows eccrypto to be used with bundlers that package Node.js app into a single executable (specifically, with nexe/nexe). There are two changes.
Avoid using return outside of a function
Node.js allows out-of-function return due to the way globals are implemented (it wraps all modules into a function), but it's not a correct JavaScript. One of the side-effects of this is that bundlers like nexe don't work with applications that use eccrypto. Bundlers use AST parsers like Acorn to parse all sources into AST and then combine them into a single JS file, and parsers choke on an out-of-function return.
Use .node extension when requiring native module
Bundlers that support bundling native modules, like nexe, need to know which require calls load native modules. To distinguish them from usual require calls, nexe looks at the extension which the require'd path ends with, and assumes it's native module if it ends with .node. Currently, eccrypto requires its native lib, ./build/Release/ecdh, without the extension, which prevents nexe from handling it specially.

Not sure why browser tests fail, it appears to be not connected with the code changes.

This needs to be rebased from 1.1 and retested, the browser tests have been fixed in the 1.1 branch.

i have problem like this error
Module parse failed: 'return' outside of function (25:4)
You may need an appropriate loader to handle this file type.
|     console.error(e);
|     console.error('Reverting to browser version');
|     return (module.exports = require("./browser"));
|   }
| }
So, i remove the return statement in node_modules before bundling.
I tried to make a PR but it was already here.
I hope this PR will be reflected soon.

I have the same error.
Can it be rebased and merged, please?


Text not centered in Roller

Hi!
I noticed that the text label of roller object type (and ddlist, too) is not really centered. It seems there is some fixed offset on the left. You can see this in the 2nd example on the webpage, too. When the text is longer, the offset does not stand out, but when there is only one letter in each option, it looks ugly.
Is this offset intentional and is there anything one can do to remove the offset?
Thanks!

Sorry: the offset is on the right side, of course.

Hi,
Thank you for pointing this out!
I fixed in the beta. Can you test it?

Hi!
Looks nice. Thanks!
BR,
Stephan


Feature suggestion: syntax for Js.Dict.t literals

I've been working with code that uses "hash map" style JS objects, and the ReScript syntax for declaring them can be tedious. (Or rather, the lack of syntax to declare them is tedious.)
Consider this JS:
const data = {
  a: foo,
  b: bar,
  c: baz,
};

Compared with this ReScript:
let data = Js.Dict.fromArray([
  ("a", foo),
  ("b", bar),
  ("c", baz),
])

That's a lot of extra parentheses and brackets to express the same thing. When you inline it somewhere (such as inside a function argument), then it's even more noisy and less readable.
My suggestion is to include a dedicated syntax similar to Js.t objects. Maybe something like this:
@dict
let data = {
  "a": foo,
  "b": bar,
  "c": baz,
}

Would this make sense to add?

I think we could even have a generic system for simple, literal keys:
// Js.Dict
dict{
  "a": 1,
  "b": 2,
  "c": 3,
}

// Belt.Map.String
map{
  "a": 1,
  "b": 2,
  "c": 3,
}

// Belt.Map.Int
map{
  1: 1,
  2: 2,
  3: 3,
}

set{1, 2, 3} // Belt.Set.Int
set{"foo", "bar", "baz"} // Belt.Set.String

That would be cool if it was possible, although that would have to still desugar/compile to function calls, right? Js.Dict can potentially compile to an object literal, which seems much simpler.

I think I saw an issue on the compiler that @bobzhang opened some time ago that discussed the possibility to convert ˋfromArray` calls with literal to the data structure itself. I think it'd be fairly easy for dicts, but not sure about Belt data structures.

@bloodyowl: Love this approach!

@bloodyowl if calls to Js.Dict.fromArray with a literal array would be compiled directly to the given json object then it would totally make sense to have a dedicated syntax. But I wouldn't mind the slight runtime cost meanwhile.
I couldn't find the issue though, are you talking about rescript-lang/rescript-compiler#2320 ? It's more than 3 years old, I hope it'll get more love now ^^

@bloodyowl I would very much be in favor of that.
My use case in case there's a better way
module Icon = {
  @react.component
  let make = (~icon: option<string>=?) => {
    Option.getWithDefault("/img/default-icon")} />
  }
}

module LabelWithIcon = {
  @module external hobbyIcon: string = "./hobby.svg"
  @module external memoryIcon: string = "./memory.svg"
  @module external standardIcon: string = "./standard.svg"
  let icons = Js.Dict.fromArray([
    ("Hobby", hobbyIcon),
    ("memory", memoryIcon),
    ("standard", standardIcon),
  ])

  @react.component
  let make = (~id: string, ~label: string) => {
    <Icon icon={icons->Js.Dict.unsafeGet(id)} />{label->React.string}
  }
}

My rationale is that explicitly typing all the possible ids with a Record would be rather tedious. Additionally by definition of this type it does not really matter if an icon displays or not, it's a nice-to-have, but no one is going to miss them if they don't display. The fromArray works but it's a bit of a roundabout way to get there so a direct syntax would be awesome!

I would still be in favor of having this feature.

I also find this interesting. @tsnobip maybe open a new issue for it in the compiler repo?

I'll transfer this one to the compiler repo.


Remove qrlImport and replace with qrl.resolve()

Problem
Currently qrlImport<T>(element: Element, qrl: QRL<T>): Promise<T> takes element. This is a problem because once we have a QRL it is hard to pass it around. There is secondary information which is the q:base of the element which can affect the behavior of QRL. To be safe one has to pass QRL and element always together to ensure that the QRL correctly serializes.
Goal
We would like to serialize and pass QRL around. This means that the QRL needs to have all of its state with it. This means that the QRL either needs to store element or it should eagerly resolve the chunk name so that the element is not necessary for the correct resolution of the QRL.
Once the QRL becomes fully self-contained, it begs the question of why qrlImport event exists? Why not just have a method on the QRL which does the same thing await qrl.resolve() (or similar).
The advantage of this is that now the QRL has full context and we can easily pass it around in code and it can be serialized in the application. The developer could write:
myFn($(() => {do something})); // <-- this reference can now be serializable

async function myFn(fn: QRL<() => void>) {
  if (something) {
    await fn.resolve()();
    await fn.invoke(); // better API which internally calls `fn.resolve()()`
  }
}

Proposed solution
Store the chunk always resolved format. (Fully qualified form).
class QRL<TYPE = any> implements IQRL<TYPE> {
  __brand__QRL__!: TYPE;
  canonicalChunk: string;
  constructor(
    public chunk: string, // <<== Make sure that is fully qalified
    public symbol: string,
    public symbolRef: null | ValueOrPromise<TYPE>,
    public symbolFn: null | (() => Promise<Record<string, any>>),
    public capture: null | number[],
    public captureRef: null | any[],
    public guard: null | Map<string, string[]>
  ) {
    this.canonicalChunk = chunk.replace(FIND_EXT, '');
  }
}

The second is to add these methods:
class QRL<TYPE = any> implements IQRL<TYPE> {
  resolve(): Promise<TYPE> {...}

  async invoke<ARGS>(..args: ARGS): Promise<ReturnValue<TYPE>> {
    return (await this.resolve)(...args);
  }
}

Fixed!


Pass variables into the template engine?

I'm porting some really really simple code from Flask and I'm wondering how to pass variables?
@app.route('/hello/')
def hello():
    return render_template('hello.html', name="Hello World")

But in crow I don't know how to pass that variable into the html. Do I have to parse it in myself?
	CROW_ROUTE( app, "/" )([]  #()
	{
		std::ifstream t( "hello.html" );
		std::string website( ( std::istreambuf_iterator<char>( t ) ), std::istreambuf_iterator<char>() );
		std::string hello = "Hello World"; // ???
		return website;
	} );

It was pretty straight forward to just parse it with boost.
I'm not familiar with crow so I don't know if this is the correct way of doing this.
std::string template_variable(std::string& website, std::string name, std::string value )
{
	try
	{
		boost::replace_all(website, "{{ " + name + " }}", value);
	}
	catch(std::exception e)
	{
		std::cerr << e.what();
	}
	return website;
}

From https://github.com/ipkn/crow/blob/master/tests/unittest.cpp:
TEST(template_basic)
{
    auto t = crow::mustache::compile(R"---(attack of {{name}})---");
    crow::mustache::context ctx;
    ctx["name"] = "killer tomatoes";
    auto result = t.render(ctx);
    // ...
}

How to create multidimension array data for mustache?
For example, I want to show list contain name of employees in a mustache template like this:

    {{#names}}

{{name}}

    {{/names}}

I don't know how to prepare array for that.
Thanks.


[Ruby 3.0] Add Ractor RBI

Add RBI signatures for Ractors.
Motivation
For completion sake, since we finished adding syntax support for Ruby 3.0, we should also provide all the necessary RBIs to go with it.
We didn't have any Ractor information, so I thought we'd add an RBI for it.

It seems the issue was outdated compiler expectation files after adding the Ractor RBI. I ran the script to update them and it seems okay now.


(v6) Best practices navigating in a thunk/saga

Hi,
I'm migrating my React/Redux app to React Router v6.
The migration guide encourages to use useNavigate instead of accesing the history object directly.
My understanding is that I could do something like this to be able to access the navigate function from my thunks:
Client side entry point:
<BrowserRouter>
  <App />
</BrowserRouter>

App.jsx:
const App = () => {
  const navigate = useNavigate();

  return (
    <ReduxProvider store={configureStore(navigate)}>
      // ...
    </ReduxProvider>
  );
}

However, I'm not sure if using the navigate function in a thunk or saga could cause conflicts with the useTransition React Router v6 uses under the hood or mess up with concurrent mode.
Could anyone provide some guidance about the recommended way for this use case?

I'm also having the same doubt, and I'm also seeing the option to migrate to the react-router. Would there be any way to perform a service so that we can browse any page after running processes in sagas?

@JoseExposito
you should try https://github.com/salvoravida/redux-first-history/releases/tag/v4.6.0-beta.1


BUG: ComponentGroup.component return trace back

🔍 Before submitting the issue

[X] I have searched among the existing issues
[X] I am using a Python virtual environment

🐞 Description of the bug
groups[0].component

📝 Steps to reproduce
from pyedb.generic.general_methods import generate_unique_folder_name
import pyedb.misc.downloads as downloads
import ansys.edb.core.database as database
from ansys.edb.core.session import launch_session
base_path = r"C:\Program Files\AnsysEM\v242\Win64"
port = 50051
temp_folder = generate_unique_folder_name()
targetfile = downloads.download_file("edb/ANSYS-HSD_V1.aedb", destination=temp_folder)
session = launch_session(base_path, port_num=port)
edb = database.Database.open(db_path=targetfile, read_only=False)
layout = edb.top_circuit_cells[0].layout
groups = edb.top_circuit_cells[0].layout.groups
groups[0].component
💻 Which operating system are you using?
Windows
📀 Which ANSYS version are you using?
No response
🐍 Which Python version are you using?
3.9
📦 Installed packages
NA

@svandenb-dev I can't reproduce this bug. Are you sure you are using the most recent version of pyedb-core? And what AEDT install are you using? I was testing with R25.2 which should be pretty much the same as R25.1 at this point.

@svandenb-dev Ah, ok I think I see what the issue is. The component property returns the ComponentGroup that the object in question is part of. In this case, that object is a ComponentGroup which is not part of another ComponentGroup. As a result, the object returned by the component property is null (note that the is_null property returns true). An assertion is thrown when a null object is accessed which is why you are seeing all the tracebacks in PyCharm. I'm going to close this out for now but let me know if there us a different behavior that is expected.


api: Add routes for accessing active community

Add the routes /api/user/active-authors and /api/user/active-authors for accessing the ids for active authors / reviewers.
Testing
Links:

http://en.serlo.localhost:4567/api/user/active-reviewers
http://en.serlo.localhost:4567/api/user/active-authors

You can change the config $mysql_timestamp_for_active_community for changing the end date of the 90 days period in which the edits / reviews are calculated. You can execute yarn mysql:import-anonymous-data and change this value to CURDATE() to test the behavior in production. The current values should be similar to

https://github.com/serlo/serlo.org/blob/master/packages/public/client/src/feature-prototypes/donation-profiles/active-reviewers.ts
https://github.com/serlo/serlo.org/blob/master/packages/public/client/src/feature-prototypes/donation-profiles/active-authors.ts

@inyono Ready to be merged :-)

@inyono new attempt :-)


Fix C decomposition rate calculation on CUDA devices

This PR resolves #61 by ensuring the decomposition rate of C is calculated using values on the CPU. It also ensures that the step size is reported as a float, regardless of the dtype and device of the problem. This is important as step size is a single value and is therefore reported like e.g. mean_eval.

Codecov Report

Merging #64 (f3e6be8) into master (eb8515b) will not change coverage.
The diff coverage is 50.00%.

@@           Coverage Diff           @@
##           master      #64   +/-   ##
=======================================
  Coverage   77.74%   77.74%
=======================================
  Files          49       49
  Lines        7321     7321
=======================================
  Hits         5692     5692
  Misses       1629     1629

Impacted Files
Coverage Δ

src/evotorch/algorithms/cmaes.py
82.94% <50.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


set_presence=off as well when app is in background

this is meant to fix vector-im/riot-android#1509
Signed-off-by: Matthias Kesler<EMAIL_ADDRESS>

https://github.com/matrix-org/matrix-android-sdk/commit/a40f38b43f24453c1484b71fde4987eaaaddb161

i prefer having a valid mIsOnline status than tricking it.


[firestore-bigquery-export] path_params is not included when importing firestore docs to BigQuery

[REQUIRED] Step 2: Describe your configuration

Extension name: firestore-bigquery-export
Extension version: 0.1.25
Configuration values (redact info where appropriate):

BIGQUERY_PROJECT_ID=${param:PROJECT_ID}
COLLECTION_PATH=organizations/{organizationId}/workspaces
DATASET_ID=comms_sandbox_firestore_export
DATASET_LOCATION=us-west3
LOCATION=us-west3
TABLE_ID=workspaces
TABLE_PARTITIONING=NONE
TIME_PARTITIONING_FIELD_TYPE=omit
WILDCARD_IDS=true

Additional configuration information

Script name: @firebaseextensions/fs-bq-import-collection
Script version: 0.1.14

[REQUIRED] Step 3: Describe the problem
When importing a collection path containing wildcard params using npx @firebaseextensions/fs-bq-import-collection, I expect the imported records to contain a path_params column containing the wildcard path params. Instead, the imported records have a path_params column equal to {} (or an error is thrown, depending on the input). This is the same problem as reported in #947 (which was erroneously closed by #982 even though that PR does not fix this issue).
Note: this only affects the @firebaseextensions/fs-bq-import-collection import script. Incremental changes synced to BigQuery via the firestore-bigquery-export extension have the correct path_params values.
Steps to reproduce:
Attempt to use npx @firebaseextensions/fs-bq-import-collection on a collection group containing wildcards. PR #982 attempted to address this issue by adding an internal resolveWildcardIds() function but that function alone doesn't address this issue. The problem is that resolveWildcardIds() expects to receive a sourceCollectionPath argument in the form regions/{regionId}/countries however the sourceCollectionPath argument is passed directly to firebase.firestore().collectionGroup(sourceCollectionPath) and .collectionGroup() cannot receive a collection ID containing "/" (indeed, an error will be thrown saying "Collection IDs must not contain '/'.").
The best solution is probably to allow Collection IDs containing "/" if the queryCollectionGroup argument is true but then call sourceCollectionPath.split("/").at(-1) to get the last piece of the collection path and pass that to the collectionGroup query.
For example:
query = firebase.firestore().collectionGroup(sourceCollectionPath.split("/").at(-1));

Additionally...
I'll also note that, as an experienced Firestore developer, I knew that collection groups couldn't receive paths in form of regions/{regionId}/countries and, for that reason, the path I originally attempted to provide to the CLI was in the form --source-collection-path=countries --query-collection-group=true. It also didn't hurt that the current documentation for the import script specifically says "You cannot use wildcard notation in the collection path (i.e. /collection/{document}/sub_collection}).".
Providing a "correct" path (i.e. without wildcards) doesn't throw an error but also doesn't properly parse the path_params. It wasn't until I was debugging the problem and creating this issue that I realized I was suppose to provide a path in the form of (regions/{regionId}/countries). So another aspect to this issue is that fact that the import script CLI needs better documentation to call out the fact that the --source-collection-path argument expects a path in the form of regions/{regionId}/countries if --query-collection-group=true. It should also be noted that, despite providing a path in the form of regions/{regionId}/countries, the import script will actually be importing all collections and subscollections named "counties" since that's how collectionGroup queries work (which some might find surprising since they are specifying a specific path like regions/{regionId}/countries).
Expected result
I expect the imported records to contain a path_params column containing the wildcard path params.
Actual result
Instead, the imported records have a path_params column equal to {}.

FYI, I patched this locally and it appears to fix the problem
I.e.
query = firebase.firestore().collectionGroup(sourceCollectionPath.split("/").at(-1));

Hey @jorroll,
Thank you for raising this issue, it's crystal clear. I added it to the project tracker for further investigation.

We would be happy to have this fix in the tool. Thanks to @jorroll  for bringing this up!

investigating this now, reproduced it.

This is very much in need of being fixed. For any application that's on production already this virtually renders BigQuery unusable for historical data. Any workaround to populate path_params column would be of critical help while this is fixed. Thanks!

I've just published a new version of the import script which should fix this, as part of a big refactor. Closing this issue now, let me know if there are any issues :)

The same issue still exists; I tested it and it is back again.


Accessibility improvements

In terms of accessibility, there are two types of inline SVG (https://www.w3.org/WAI/standards-guidelines/act/rules/7d6734/)
Informative icon:

Icon with title (and possibly desc) tags.
Icon with label attributes such as aria-label or aria-labelledby.

Informative icon needs an extra "role" attribute.
Decorative Icon

Icon intended for decoration.

Decorative icon needs an extra "aria-hidden" attribute.
Suggestion
At present, these attributes can be set "manually".
My request is to add some default attributes if a label and/or children tags are set or not.
Something like (I'm using the React template, I can open a pull request if needed) :
const createLucideIcon = (iconName: string, iconNode: IconNode) => {
  const Component = forwardRef<SVGSVGElement, LucideProps>(
    ({ color = 'currentColor', size = 24, strokeWidth = 2, absoluteStrokeWidth, children, ...rest }, ref) => {
      const attributesRelatedToA11y = children || rest['aria-label'] || rest['aria-labelledby']
        // https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/img_role#svg_and_roleimg
        ? { role: 'image' }
        : { ['aria-hidden']: true };
      return createElement(
        'svg',
        {
          ref,
          ...defaultAttributes,
          ...attributesRelatedToA11y,
          width: size,
          height: size,
          stroke: color,
          strokeWidth: absoluteStrokeWidth ? Number(strokeWidth) * 24 / Number(size) : strokeWidth,
          className: `lucide lucide-${toKebabCase(iconName)}`,
          ...rest,
        },
        [
          ...iconNode.map(([tag, attrs]) => createElement(tag, attrs)),
          ...(
            (Array.isArray(children) ? children : [children]) || []
          )
        ],
      )
    },
  );

NB: If a user calls an informative icon and defines an explicit role such as role="graphics-document", it will replace the default role="image"

In part a duplicate of #477


Mobile - Account summary - notifications view

Figma:
https://www.figma.com/file/bZh7tOiLm6Ls9Hl4RL6qMC5X/Augur-Redesign?node-id=7187%3A250630
NB review with Cillian before building.
Notes:

Notifications are accessed using the tabs at the top of the page
IF there are new notifications, show red dot (beside tab link). IF there are no new notifications, hide red dot
Tapping a notification opens a modal (a separate ticket will be created for these)

Except for reporting/disputing notifications, which will link directly to the relevant page for the moment

@andrewdamelio is there any dev wallet we use that will allow me to see all the notifs?

@cillianhunter0 i'll msg you some accounts on discord


The default static folder is being ignored if the number of params to the route are 3

@app.route('///path:filename')
@login_required
@nocache
def custom_static(filename, username, date):
path = os.path.join(app.config['CUSTOM_STATIC_PATH'],"username", username, date)
print "Path: {}".format(path)
if os.path.isfile(os.path.join(path, filename)):
return send_from_directory(path, filename)
else:
print "File does not exist: {}".format(filename)
return
Path: ./DataWebserver/username/js/static
File does not exist: bootstrap-datepicker.min.js
enter code here

Please provide a minimal reproducible example, not a random piece of code from your software.


Unit Tests

Currently, we have no unit tests at all - we should have a test for each method.

This is complete


Upgrade Go version to 1.22 for the v1 branch

The dependency github.com/distribution/distribution/v3 is now using Go 1.22.

Closing as #790 has been merged.


"cpu" or cpu not supported as device in settings_eval.ini

When i set device to "cpu" in settings_eval.ini, just to test cpu performance, i get:

Traceback (most recent call last):
File "run.py", line 187, in
evaluate_bulk(settings_eval['EVAL_BULK'])
File "run.py", line 136, in evaluate_bulk
mc_samples)
File "/home/diedre/git/quickNAT_pytorch/utils/evaluator.py", line 260, in evaluate
model.cuda(device)
File "/usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py", line 311, in cuda
return self._apply(lambda t: t.cuda(device))
File "/usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py", line 208, in _apply
module._apply(fn)
File "/usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py", line 208, in _apply
module._apply(fn)
File "/usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py", line 230, in _apply
param_applied = fn(param)
File "/usr/local/lib/python3.6/dist-packages/torch/nn/modules/module.py", line 311, in
return self._apply(lambda t: t.cuda(device))
RuntimeError: Invalid device, must be cuda device

Seems like an easy fix, change of using model.cuda(device) to model.to(device), with device being redefined as something like:
torch.device("cpu") if device == "cpu" else torch.device("cuda:{}".format(device))
I also tested cpu, and got:

Traceback (most recent call last):
File "run.py", line 186, in
settings_eval = Settings('settings_eval.ini')
File "/home/diedre/git/quickNAT_pytorch/settings.py", line 10, in init
self.settings_dict = _parse_values(config)
File "/home/diedre/git/quickNAT_pytorch/settings.py", line 27, in _parse_values
config_parsed[section][key] = ast.literal_eval(value)
File "/usr/lib/python3.6/ast.py", line 85, in literal_eval
return _convert(node_or_string)
File "/usr/lib/python3.6/ast.py", line 84, in _convert
raise ValueError('malformed node or string: ' + repr(node))
ValueError: malformed node or string: <_ast.Name object at 0x7f13134c55c0>

The code probably expects an int or string.

I am getting a similar error, when trying to run the eval_bulk command. The error is:

RuntimeError: Attempting to deserialize object on a CUDA device but torch.cuda.is_available() is False. If you are running on a CPU-only machine, please use torch.load with map_location=torch.device('cpu') to map your storages to the CPU.

Following community suggestions, I modify line 33 in run.py to the below, but I am still getting the same error.
   final_model_path = os.path.join(common_params['save_model_dir'], train_params['final_model_file'])

Altering utils/evaluator.py gives the desired functionality (starting line 256):
   cuda_available = torch.cuda.is_available()     if cuda_available:         model = torch.load(coronal_model_path)         torch.cuda.empty_cache()         model.cuda(device)     else:         model = torch.load(coronal_model_path, map_location=torch.device('cpu'))


Multitargeting support in the Razor editor

Multitargeting is becoming more common in projects that contain Razor:

.NET MAUI Blazor Hybrid apps use Razor and target multiple mobile & desktop platforms.
We are exploring adding a browser target framework moniker in .NET8 to enable targeting running in browsers on WebAssembly

Razor today doesn't really have any support for multitargeting. We are already tracking adding Razor language support for multitargeting here: https://github.com/dotnet/razor/issues/7629
We also need to add support for multitargeting in the Razor editor. Currently none of the drop downs show up when working with Razor files:
Here are the drop downs you get in a C# file:

You don't get the same drop downs in Razor:

@davidwengier @phil-allen-msft Should this be closed as a duplicate of https://github.com/dotnet/razor/issues/8983 at this point?

Makes sense to me. Also worth nothing https://github.com/dotnet/razor/issues/4341 for adding the dropdowns is related, and has been completed.


No description for item 2071: "TARGET_LIST"

When importing PDSDD, we always get the warning:
No description for item 2071: "TARGET_LIST"
We should find a way to fix this.

TARGET_LIST really doesn't have a definition in pdsdd.full, and that hasn't been fixed in the latest release. The warning is correct.


MSID: 95406 Version: 2 DOI: 10.1101/2<IP_ADDRESS>6886

MSID: 95406
Version: 2
Preprint DOI: https://doi.org/10.1101/2<IP_ADDRESS>6886
Step 1. Awaiting reviews
Editorial to post reviews via hypothesis
Useful links:

DocMap: https://data-hub-api.elifesciences.org/enhanced-preprints/docmaps/v2/by-publisher/elife/get-by-manuscript-id?manuscript_id=95406
New model tracking: https://docs.google.com/spreadsheets/d/1_fHaoOy7hjyocptKtVJRijeNpUY4hBS7Ck_aVmx6ZJk/
Reviews on sciety: https://sciety.org/articles/activity/10.1101/2<IP_ADDRESS>6886

For trouble shooting (e.g. no Docmaps available):

DocMap issue addressing: https://miro.com/app/board/uXjVNCwK6EI=/
Explore DataHub DocMaps API: https://lookerstudio.google.com/reporting/4c2f0368-babb-4beb-b5b3-497e7e7b0f08/page/ejphD
Unmatched submissions and preprints: https://lookerstudio.google.com/u/0/reporting/9f86204f-3bf7-477c-9b18-5c5ef141bf69/page/p_gxi57ha93c
Unmatched manuscripts spreadsheet: https://docs.google.com/spreadsheets/d/15QcK8w-ssB7109RQEDtFpJPZ0J5HTGxoHa_2TtpMBbg/edit#gid=1336081641

Step 2. Preview reviewed preprint
Production QC content ahead of publication
Instructions:

QC preview: https://prod--epp.elifesciences.org/previews/95406v2
Update ticket with any problems (add blocked label)
When QC OK, add QC OK label to ticket and add publication date and time to https://docs.google.com/spreadsheets/d/1amAlKvdLcaDp5W8Z8g77NmkwbMF5n_u89ArSqPMO8jg
Move card to next column
(At end of the day post link in #enhanced-preprint and ask for PDF to be generated)

Useful links:

Preprint DOI :  https://doi.org/10.1101/2<IP_ADDRESS>6886
Confirm reviews returned by EPP: https://prod--epp.elifesciences.org/api/reviewed-preprints/95406/v2/reviews
To update the MECA path in the docmap: https://docs.google.com/spreadsheets/d/1mctCQuNFBjSn97Lihy7_vBO6z7-N-oqyLv4clyi6zHg

Step 3: Awaiting search reindex
This step adds the reviewed preprint to the homepage: https://elifesciences.org
The search reindex is triggered once an hour. We need the reviewed preprint to be indexed as the search application serves the journal homepage.
Useful links:

Jenkins pipeline to reindex search can be triggered sooner or monitored here: https://alfred.elifesciences.org/job/process/job/process-reindex-reviewed-preprints/

Step 4: Published! PDF requested
Waiting for PDF to be generated
Useful links:

PDF tracking: https://docs.google.com/spreadsheets/d/106_XeDjmuBae7gexOTNzg60lapeqjl2aRn9DzupGyS8/

Step 5: Introduce PDF to data folder and git repo
Upload PDF to relevent folder in git repo https://github.com/elifesciences/enhanced-preprints-data/
Step 6: Done!

[ ] Kettle is on!

error: [journal-ref-article-title] This journal reference (id c45) has no article-title element.
warn: [mixed-citation-other-publication-flag] mixed-citation in reference (id=c27) has a publication-type='other'. Is that correct?
warn: [mixed-citation-other-publication-flag] mixed-citation in reference (id=c30) has a publication-type='other'. Is that correct?
warn: [mixed-citation-other-publication-flag] mixed-citation in reference (id=c53) has a publication-type='other'. Is that correct?
warn: [mixed-citation-other-publication-flag] mixed-citation in reference (id=c70) has a publication-type='other'. Is that correct?

@fred-atherden I'm running this locally and I get completely different schematron outcome. This only has 22 references!

Yeah, sorry, I copied it into the wrong ticket


Error: Heartbeat timer is already running

When I startup libp2p-gossipsub as part of libp2p I get unhandled promise rejection error as follow:
(node:77341) UnhandledPromiseRejectionWarning: Error: Heartbeat timer is already running
    at Heartbeat.start (/Users/adam/Projects/iovlabs/rds-ipfs/node_modules/libp2p-gossipsub/src/heartbeat.js:41:27)
    at Gossipsub.<anonymous> (/Users/adam/Projects/iovlabs/rds-ipfs/node_modules/libp2p-gossipsub/src/index.js:340:28)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/adam/Projects/iovlabs/rds-ipfs/node_modules/libp2p-gossipsub/src/index.js:24:58)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Do you have some ideas why is it happening? Can it affect something?
Currently, it seems that everything works as expected even with this error, but it raises concern...

Hello @AuHau
Can you let me know the versions you are using?
It seems that gossipsub.start is called multiple times

Sure, here: https://github.com/rsksmart/rif-communications-pubsub/blob/master/package.json#L45

So, something is triggering the second start of heartbeat. Nothing bad should be happening because it is already running.
https://github.com/ChainSafe/js-libp2p-gossipsub/blob/v0.4.6/ts/heartbeat.ts#L30
https://github.com/ChainSafe/js-libp2p-gossipsub/blob/v0.4.6/ts/index.ts#L382
We are actually removing this error from the new gossipsub code and just return if already running. However, the start should not be called twice. When you do libp2p.start() it calls pubsub.start that calls hearbeat.start. I could only find in your code libp2p.start, which seems correct. It would be nice if you could do a console.trace() in the source of gossipsub before the throw, so that we can see where the second start is coming from

That is great idea! Should have thought of that, will try. Thanks!

Hmm the trace is not much helpful as it is pretty much the same as in the first message for both of calls. I have putted before the start() so yes there are two call but I think they should be from different libp2p instances.
But made me think. Is it alright to spawn multiple libp2p instances? We need those for tests and maybe there is some "singleton" pattern here that leads to this duplicated call?

But made me think. Is it alright to spawn multiple libp2p instances?

Yes, but they should have different peerIds. It is normal to have multiple logs if you have multiple peer ids. You can also log the peer Id there, IIRC that version should have a gossipsub.peerId or gossipsub._peerId property

So after bit more digging in our code, I have actually discovered that we were actually calling start() twice, so that was the cause 😅
But you actually helped me debugging other problems I had with your last suggestion so, thanks a lot!


Do not use private static attributes for instrumentation of Interfaces

Suggested Solution
Use static instead of private static
Note
Might already be implemented and the implementation of Peass might be responsible for the false instrumentation.

Thanks for the issue.
This should already work, as checked by https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/main/kieker-source-instrumentation-library/src/test/java/net/kieker/sourceinstrumentation/it/SourceInstrumentationInterfaceIT.java (and the example interface is in https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/main/kieker-source-instrumentation-library/src/test/resources/project_2_interface/src/main/java/de/peass/SomeInterface.java).
I do not see that there is a special configuration for this, the KiekerFieldAdder just adds static final instead private final on all interfaces: https://github.com/DaGeRe/kieker-source-instrumentation-dagere/blob/f37cd0d1acb381470315a95a993ca9a3d6b92692/kieker-source-instrumentation-library/src/main/java/net/kieker/sourceinstrumentation/instrument/KiekerFieldAdder.java#L68
Might your concrete use case contain other aspects, e.g. inner classes or something like this? Could you provide the full class where the problem occurs?

The error only occurred for measurement instrumentation of root cause analysis. An example interface with that error was https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/JarScanFilter.java

For instance, I executed:
./peasstomcat searchcauseParameterized 314568fa450680711f12f01ee050f2166f4263bd org.apache.catalina.connector.TestResponse#testSetLocale04
For that command, the error occurred in the file https://github.com/stro18/tomcat/blob/314568fa450680711f12f01ee050f2166f4263bd/java/org/apache/coyote/ProtocolHandler.java

The following lines caused the error:
private static int _kieker_sourceInstrumentation_createCounter0;

private static long _kieker_sourceInstrumentation_createSum0;

Thanks for the command, with this command, I can reproduce the problem. I'll fix it in the next days.

I confirm that it works.


Add/remove spaces

Docs Build status updates of commit 880a88f:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

sysinternals/downloads/movefile.md
:white_check_mark:Succeeded
View

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


When bounds are provided, also obtain a lat-lng point to bias autocomplete results with

hey @louh I'm just cleaning up this Milestone, could you provide more info on this issue, is it a big priority?
as I read it, it means:

when the client sends the focus.viewport.* params it should also send focus.point.* params.

If that is the case then it's a low priority as the backend API currently just computes the centroid for you

Hey @missinglink, I don't know how much autocomplete has changed in the last few weeks. Your reading is correct, but I'm writing it with the assumption that /autocomplete does not understand boundary.rect.*, and so the geocoder plugin itself should calculate the appropriate focus.point.* client-side. Is focus.viewport.* new? (or undocumented?)

I found the answer to the focus.viewport question. https://github.com/pelias/api/pull/620 (SPOILER: it's gone)

Closing this. The code to write this would be complicated & with too many edges (e.g. a bounds is provided but focus is set explicitly to false), and it would still not answer any questions about why autocomplete results (#131, #108) do not respect bounds. As questions come up we need to instruct developers to provide both bounds and focus options separately for search and autocomplete respectively.