Add (experimental) support of java.lang.Comparable

Migrated from Google Code (issue 439)

Proposal:

We deprecate @EqualsAndHashCode (but it'll never be removed, really, or at least not for a long while).

Using both @EqAHC and @Ordered at the same time is invalid.

@Data and @Value switch over from implying @EqAHC to implying @Ordered (without comparable=true!)

We create a new annotation, @Ordered. It broadly fills the same role as @EqAHC.

of and exclude go away. comparable = true/false (default = depends on whether you added implements Comparable to your implements list) is added. onParam is renamed to onEqualsParam, and onCompareToParam is added (oCTP without comparable=(default) true is an error).

the same batch of fields is used for all 4 methods (canEqual, equals, hashCode, and compareTo).

For comparable=true, any field so included must either be a primitive (we know how to compare this intrinsically), or must implement Comparable. We don't check this; we just call .compareTo(o), and if that's not a valid action the compiler will complain about generated code. This means we do not have to do resolution which is a good thing.

Trying to compare arrays is a lombok-generated error; you must specify a comparison method/comparator class or exclude the field.

If comparable=true, we don't just generate compareTo, we also add implements Comparable<SelfType> to the implements list if it isn't already there.

To configure individual fields, add sub-annotations of ordered:

@Ordered.Include. If present, any use of @Ordered.Exclude anywhere in this class is invalid. All fields without an @Ordered.Include are excluded if any field is so annotated.
@Ordered.Exclude. Nuff said.
@Ordered.NullOrder(NullOrder.NULL_IS_MAX -or- NullOrder.NULL_IS_MIN -or- NullOrder.NULL_IS_ERR). Should the default behaviour be NULL_IS_MIN, or NULL_IS_ERR?
@Ordered.DoNotUseGetter; overrides the doNotUseGetters of the @Ordered annotation.
@Ordered.Use(method = "methodName") - this is unfortunately still stringly typed, but there's just no way to tie a method to a field without stringly typing one of them. Alternatives are magic names (if the field is named foobar, then a method named foobar_compare is assumed to be the comparer method); this sounds worse than specifying it in the annotation. At least there we can error if there's no method with that name. The method can return either boolean or int; if int, negative = first arg is lower, 0 = equal, positive = first arg is larger. If boolean, an error is generated unless comparable=false. The method must take 2 arguments.
@Ordered.Use(comparator = ComparatorClass.class) - this will instantiate ComparatorClass with the no-args constructor and then call .compare(a, b) on it, assuming that it is a Comparator implementation.
@Ordered.Reverse. Nuff said.
@Ordered.Order(int). By default all fields have the '0' order. All the 4 generated methods will start out with the fields that have the lowest order number. If multiple fields have the same order, they are handled in the order they show up in the source file, as now.

Like @EqAHC now, static fields, transient fields, and fields that start with $ are skipped.

primitives get special treatment. arrays also get special treatment, but for compareTo that's just lombok generating an error; you must exclude these or specify an explicit comparator with @Ordered.Use.

OPEN ISSUES:

default NULL_IS_MIN or NULL_IS_ERR?
Go with all these sub-annotations, or have one @Ordered.Conf annotation that has all that stuff as params instead?
Is this the right way to go about specifying custom equality / comparator methods?
How should we handle the process of deprecating @EqAHC?
Is the general concept of 'always treat equals, hashCode, canEqual, and compareTo as sides of the same coin' okay? There are some indications that java code out there uses different fields, but this doesn't sound like it can adhere to the rule that if a.equals(b), then a.compareTo(b) == 0 and vice versa.
You now can't refer to expressions that are a comparator; you can't use this to specify String.CASE_INSENSITIVE_COMPARATOR for example. Kind of annoying. If only fields or methods could be referenced in annotations!

I would recommend NULL_IS_MIN as default to lower the chances of accidental exceptions.
To improve readability I would go for a single annotation. However, I would prefer this annotation to be at field level instead of a sub-annotation.
I would leave @EqualsAndHashCode for now. Even deprecating it would cause problems for long-time users.
For equals/compareTo see below.
There should be an option to specify a class that implements Comparator (instead of a method name). Unfortunately you can't specify class instances like String.CASE_INSENSITIVE_COMPARATOR, so for strings I recommend also adding @Ordered.CaseInsensitive / caseInsensitive="true".

Should equals and compareTo == 0 be identical? e.g.
A. When a.equals(b) then a.compareTo(b) == 0
B. When a.compareTo(b) == 0 then a.equals(b)
The Java API does not have this restriction AFAIK.
Classes like HashMap and HashSet use equals() and hashCode() exclusively and classes like TreeMap and TreeSet only use compareTo. I can't find any class that uses both compareTo and equals.
My classes adhere to (A) but not to (B) and I have never experienced any problems with this!

My 2ec:
.- I'm not a big fan of having so many different annotations, I'd rather have less annotations (one if possible) with multiple parameters.
.- From the usubility point of view, I think some people might miss the name Ordered when looking on how to configure the equals/hashcode behaviour. For example, I have many entities I might use as keys in maps or just in sets, but they don't require any order, so I simply configure the real keys in EqAHC. Having an annotation named Ordered for that does not sound intuitive to me. Maybe something lilke @Identity? Not sure if being so similar to javax.persistence.Id that has similar goals is good or bad :D

Just some random and partly conflicting ideas. Please let me know which ones you like.

How should we handle the process of deprecating @EqAHC?

Maybe not at all? Later, there may be additional options for @EqAHC (e.g. precomputed or cached), which would not fit that well. Maybe we should keep it:

Add @Ordered as stated above and let it by default use the same fields as @EqAHC.
Deprecate include and exclude, add @EqAHC.Include and @EqAHC.Exclude (independent feature).
Use @Ordered.Include and @Ordered.Exclude for the cases when they use different fields (independent feature).

A big advantage of independent features is that someone could rather easily contribute them (generating methods and other magic is much harder than such modifications).

@Ordered.NullOrder - this should be allowed on the whole class and also in the configuration. Then the default doesn't really matter.
Ideally, combining NullOrder.NULL_IS_ERR with @Nullable should give a compile time error, but this is probably impossible, because of the fuzziness of @Nullable.
IMHO NULL_IS_ERR without lombok.@Nonnull should generate a warning, unless given explicitly on the field (then the user is supposed to know what they're doing).

Maybe there should be @Ordered.Special(booleans=FALSE_FIRST, strings=CASE_INSENSITIVE, arrays=LEXICOGRAPHICAL), or something like this making the common cases simpler.

@Ordered.Use(method = "methodName")

I guess, you want to use the boolean to tell if the fields are equal or not; true means "equal", but it's not obvious.
This could get cleaner with something like compareTo="methodName" and/or equals="methodName" and/or hashCode="methodName".

@Ordered.Use(comparator = ComparatorClass.class)

I'd maybe replace comparator by valuein order to make the more common (and nicer) case simpler to use.

default NULL_IS_MIN or NULL_IS_ERR?

I'd suggest the latter, overridable with a configuration key and a class-wide option.

When useGuava=true is on, use Guava ComparisonChain to make the generated class small.

I couldn't help myself and tried to make my ideas into a complete proposal.
Keeping @EqAHC leads IMHO to a clearer design. I added some options to it for completeness.
I'd use @CompareTo instead of @Ordered as it's more obvious for people used to @EqAHC and avoids the rather confusing @Ordered.Order(int) and a clash with org.springframework.core.Ordered. I'm sticking with @Ordered below.
Summary

@EqAHC

keep @EqAHC
(optional) add @EqAHC.Include and .Exclude, deprecate include and of
(more optional) add @EqAHC.Use(SomeEquivalence.class) where SomeEquivalence

has a no-args constructor
implements the two abstract methods doEquivalent(T, T) and doHash(T) of com.google.common.base.Equivalence (without necessarily extending it)

(more optional) add @EqAHC(precomputed=boolean, cached=boolean) where the options are mutually exclusive

@Ordered (or better @CompareTo)

add @Ordered using by default the same fields as @EqAHC uses (or would use) in the source file order
(optional) add @Ordered.Reverse

on fields meaning to reverse the sign of the single comparison
on classes meaning to reverse the sign of the result

(optional) add @Ordered.Order(int)
(optional) add @Ordered.Use(value=SomeComparator.class, compareTo="someMethodName") where always exactly one option must be used
(optional) add @Ordered.Special(value=Ordered.SpecialPolicy.XXX) allowing to specify multiple enum members

Details
@EqAHC
Out of the four possibilities for specifying fields, at most one can be used per class.
@EqAHC.Use(SomeEquivalence.class) is compatible with Guava without depending on it. The user has to implement the two methods as if they were extending Equivalence, but they may or may not extend it. IMHO linking to doEquivalent and doHash is way better than any own specification.
Obviosly, equivalent and hash sound better, but doEquivalent and doHash are the core methods freed of boilerplate (identity and null checks).
@Ordered.Special
This would allow the following options (names partly taken from Guava Ordering)

important

NULLS_FIRST
NULLS_LAST
NULLS_THROW

rather important

FALSE_FIRST
TRUE_FIRST
CASE_INSENSITIVE
CASE_SENSITIVE

fancy

LEXICOGRAPHICAL

It's a bit unstructured, but rather practical. Some combinations make sense, many do not, but for a String[] you can use
@Ordered.Special(specials={NULLS_FIRST, CASE_INSENSITIVE, LEXICOGRAPHICAL})

and the meaning is rather obvious (apart from NULLS_FIRST being applied to both the array and its members).
All nonsensical or not yet implemented combinations must raise an error.
The target should be fields and classes.
Comments

default NULL_IS_MIN or NULL_IS_ERR?

Doesn't matter much, when a corresponding configuration key and a class-wide option exist (and a warning is given for fields throwing on null and having neither @NonNull or NULLS_THROW on them).

Go with all these sub-annotations, or have one @Ordered.Conf annotation that has all that stuff as params instead?

A single annotation wouldn't work well because of its target (some make sense on fields only, others on both).
Even with all the fancy and optional stuff, the number of annotation is not that high and you hardly ever need more of them. Sure, there could be
@Ordered(reverse=true, nulls=NULLS_FIRST, strings=CASE_INSENSITIVE, booleans=FALSE_FIRST)

on a class and
@Ordered.Conf(exclude=false, order=2, use=SomeComparator.class)

on a field and maybe it's a good idea. I'd start with plain dumb @Ordered() and watch the demand.

Is the general concept of 'always treat equals, hashCode, canEqual, and compareTo as sides of the same coin' okay? There are some indications that java code out there uses different fields, but this doesn't sound like it can adhere to the rule that if a.equals(b), then a.compareTo(b) == 0 and vice versa.

This rule is "strongly recommended, but not strictly required", so a possibility to violate it should not be needlessly rejected. In such cases, I prefer to implement a Comparator instead, but if I could use a generated compareTo, I'd do it. By not conflating @EqAHC and @Ordered, the possibility remains open with an obvious syntax.

Two remarks:

I really don't like the annotation name<EMAIL_ADDRESS>I would recommend simply naming it @Compare (and the one at class level @CompareTo as suggested).
We don't need a "booleans" attribute. I would recommend using FALSE_FIRST as the default and using reverse="true" if you want TRUE_FIRST.

There is something to be said about allowing both a nested class-level annotation and a field-level annotation for @Ordered.Conf. Some users like a clutter free class body and some like annotations to be as closely to the object it is related to (I prefer the latter).
This would mean that @Ordered.Conf needs a default "value" attribute to specify the field name (not needed when specified on a field).
It should also be possible to specify @Ordered.Conf on a getter method instead of a field (in case some extra calculations are required on the value).

Roel and I had a long talk on this and we did not reach any concensus. The major issues we just can't fix are:

We REALLY want to combine compareTo and equals/hashcode because they are almost always built the same way. NOT combining them results in serious issues when trying to include/exclude fields. Should @Ordered piggyback on @EqAHC's includes and excludes lists UNLESS @ordered has it's own? That's just not intuitive AT ALL, it's a vetoed non-starter here. If we combine them, what's the name? Identity was shot down because it often means the inverse (as in IdentityHashMap), Ordered is shot down because it is too suggestive of comparables. The final winners of the evening were some random placeholder name (@Ecco) or @Eq. This would be the annotation that does both.

Use in general shot down because it needs to know how to calculate hashCode too. That means we're indeed stuck with needing a class that implements / looks like guava's Equivalence, except we can't use that either, because we need a comparable result (negative/zero/positive), and that's not what Equivalence does. So now it's a huge mess that's stuck together with nonsense and structural typing, none of which is very 'java'. We currently have no non-vetoed plan to do anything for the concept of 'use', which is obviously bad news. Best bet is to specify a class that implements a lombok-specific class that MUST be local, at which point we can strip out the 'implements' part there so as to ensure that lombok is not needed at runtime. This doesn't sound right either.

An alternate plan of sticking with 2 annotations (@Eq and @Ordered most likely) runs into trouble with specifying details. We want a single exclusion/inclusion system, how do we do this? There are a number of settings that are only relevant to just one of those two, and others that are relevant to both. How do we handle that? One massive top-level @FieldSettings might work, but this becomes the clowncar of settings, as it would also have to contain @ToString's exclusions and settings (such as 'skipNulls' which optimally should be per-field configurable). This clown car plan doesn't sound very scalable or pretty either.

We ran around in circles and didn't get anywhere with a design that felt anywhere even close to right.
This proposal seems to be sinking for want of a name :/

What is the reason you want to include compareTo in @EqualsAndHashCode? Not every class that has a equals/hashCode needs a compareTo, so I would not have a problem with a separate @CompareTo annotation.
A @FieldSetting annotation also doesn't seem a good idea to me. Do you really need a single exclusion/inclusion system? I agree that the goal should be to be as concise as possible, but separating the settings into several annotations (e.g. @EqualsAndHashCode.Field, @ToString.Field and @CompareTo.Field) would make the concept easier to understand to first-time users and lower the learning curve.

We want to combine them because, IF want compareTo, you also want equals/hashCode, and almost always, you want the same batch of fields to define the  behaviour of all those methods.
We want one annotation; that doesn't mean the annotation always generates compareTo. That'd be a choice. But the same annotation powers both of it (you can choose between generating equals/hashCode/canEqual, or equals/hashCode/canEqual/compareTo. Those are the only 2 choices).
Yes, we need a single exclusion/inclusion system, for all the reasons explained earlier. @EqAHC.Field and @Ordered.Field, while easy to understand, is therefore not feasible without an expansive analysis of how to marry the inclusion/exclusion lists.

I understand why you would want to combine equals/hashCode and compareTo, but you concluded yourself that this would cause almost too much trouble with the nested field configuration.
Maybe @CompareTo could borrow the field includes/excludes from @EqualsAndHashCode by adding an attribute like useEqualsAndHashCodeFields (with "true" being the default value).
That way you could easily use the same includes/excludes, but specifying a different set of includes (like I would be doing) would also be very easy.

Here you have some examples of what I'm suggesting:
Using nested annotations:
@EqualsAndHashCode(
	callSuper = true,
	fields = {
		@EqField(name = "id", doNotUseGetter = true),
		@EqField(name = "extra", exclude = true),
	}
)
@CompareTo(
	fields = {
		@CompField(name = "name", caseInsensitive = true)
	}
)
public class Test {
	private int id;
	private String name;
	private Object extra;
}

and
@EqualsAndHashCode(
	exclude = {"extra"}
)
@CompareTo(
	useEqualsAndHashCodeFields = false,
	fields = {
		@CompField(name = "id"),
		@CompField(name = "extra", comparator = CompareExtra.class),
	}
)
public class Test {
	private int id;
	private String name;
	private Object extra;
}

and/or using field-level annotations:
@EqualsAndHashCode(callSuper = true)
@CompareTo()
public class Test {

	@EqField(doNotUseGetter = true)
	private int id;

	@CompField(caseInsensitive = true)
	private String name;

	@EqField(exclude = true)
	private Object extra;
}

and
@EqualsAndHashCode()
@CompareTo(useEqualsAndHashCodeFields = false)
public class Test {

	@CompField()
	private int id;

	private String name;

	@EqField(exclude = true)
	@CompField(comparator = CompareExtra.class)
	private Object extra;
}

What if you provide just one annotation (EqualsAndHashCodeAndCompareTo) which implements a compareTo method based on the fields configuration of the annotation and this compareTo is just used in equals as well (return 0 == this.compareTo(other))

Some more ideas...

Should @Ordered piggyback on @EqAHC's includes and excludes lists UNLESS @Ordered has it's own? That's just not intuitive AT ALL

Then forget it and use 3 annotations: @EqAHC, @Ordered, and @EqOrdered. Let the last one govern common options and let it be overridable by the specific ones. Use also corresponding field level annotations. This is a bunch of annotations, but their number does not make anything worse.

Ordered is shot down because it is too suggestive of comparables.

Sure, @Ordered generating no compareTo by default sounds wrong, so what about @EqOrdered?

Yes, we need a single exclusion/inclusion system, for all the reasons explained earlier. @EqAHC.Field and @Ordered.Field, while easy to understand, is therefore not feasible without an expansive analysis of how to marry the inclusion/exclusion lists.

Maybe our includes/excludes thinking is a part of the problem. The two things are symmetrical, but only as long as you ignore configuration. When you exclude a field, then there's nothing more to configure. So maybe there should be nested @*.Exclude (parameterless) and @*.Include (with some parameters) annotation. Of course, you can use @*.Field(exclude=true) and @*.Field(option1=..., option2=....) instead of the two annotations. Combining them is trivial:

a field level annotation wins over class level annotation
otherwise, @EqAHC and @Ordered win over @EqOrdered
combining @X.Include and @X.Exclude with the same X is nonsensical and therefore forbidden (in case of sticking with @*.Field: combining exclude=true with any other option is forbidden).

Also add includeNone, which excludes all fields by default (so only those included on field level get in).
Let me try:

keep @EqAHC, deprecate include and of, add includeNone=false
add @Ordered(reverse=false, includeNone=false) (which just generates compareTo)
add @EqOrdered(includeNone=false) as a shortcut for both @EqAHC and @Ordered
add @EqAHC.Exclude, @Ordered.Exclude, and @EqOrdered.Exclude
add nested *.Include field level annotations allowing to specify details:

@EqAHC.Include(equivalence=..., order=...)
@Ordered.Include(reverse=..., comparator=..., order=...)
@EqOrdered(order=...)

Somewhat related. I dream of someday being able to write
public @NamedTuple class Foo {
    private int bar;
    private String baz;
}

and get a Java class that I would resemble a python namedtuple. In addition to the current @Data, a @NamedTuple would provide map like access to the data elements.
Rough sketch
Foo foo = Foo(1, "str")
foo.entrySet() == [ "bar", "baz" ]
foo.get("bar") == 1
for (Object value : foo.values()) {
    ...
}
for (Entry e : foo.entrySet()) {
    ...
}

More to the point
We can look at how other languages deal with this. In Python tuples are sorted lexicographically (first `bar, then baz above). If some other ordering is required, the general practice seems to be to define a new Comparator as required.
As for the name of the new annotation, I offer @CompareEqualsAndHashCode, with an optional comparator argument.

Hi, I'd like to see this in future versions. Please add a milestone ;).

We're picking this idea back up. For further discussion see https://github.com/rzwitserloot/lombok/wiki/FEATURE-IDEA:-Annotation-based-Include-Exclude-for-ToString,-EqualsAHC,-etc


Slides for #72 off by one++

@MarlonSchultz are you set up to host this from your forked github.io page?

@lucasrangit I always build it locally on my machine via docker and present it from there. Dont have github pages set up.

@lucasrangit I tend to approve this one, even though the .idea entry is only useful for IntelliJ users.
Reason: The alternative would be forcing people to keep local changes in their repos, which is never a good thing.
The situation would be different if Git had support for some kind of global .gitignore files (e.g. some dot-file in $HOME), but I'm not aware of such a Git feature.

merging this now - but I tend to agree with @vog regarding the .idea folder. Let's discuss this at next BHNT. But I do not think it is a blocker to merge as it is not a big deal - so merging this now


influxd backup /tmp/mysnapshot   gives an EMPTY backup

I launched influxd backup /tmp/mysnapshot  following help manual
https://influxdb.com/docs/v0.9/administration/backup_and_restore.html
on the last version (which I got by running
wget http://influxdb.s3.amazonaws.com/influxdb-0.9.2-1.x86_64.rpm
sudo rpm -ivh influxdb-0.9.2-1.x86_64.rpm
Result:  the file /tmp/mysnapshot is simpy EMPTY.

@ibobak by default snapshotting is disabled. If you'd like to enable snapshotting add
[snapshot]
enabled = true

to you influxdb config file.

then why not to output a message  "snapshots are disabled"  when I am doing "influxd backup" ?

@ibobak yeah thats probably a good idea. Open up an issue for it.


thread 'main' has overflowed its stack

i face this iuse "thread 'main' has overflowed its stack"
clap v2.34.0 this clap version

clap is now bumped to 3.2.8 in master - does that help?
If not, share more diagnostic info about the failure.

@dholroyd could you please output a new release that has this package bumped? I installed scte35dump today using cargo install scte35dump and it still is on version 0.1.6, which is using clap 2.34.0 still. It looks like that 0.1.6 release was made long before the upgrade of clap to 3.2.8.
In short, on Windows after a fresh install, running scte35dump --help results in the response thread 'main' has overflowed its stack.

@turbidwater I've bumped several dependencies to even more recent releases and pushed version 0.1.7.  Please give it a spin!

Looks like this problem with clap will need more investigation.
In the meantime, please can you try building an exe with a larger default stack as a workaround?
I was able to reproduce this problem in a VM, and was able to work around it with the following steps,
set RUSTFLAGS=-C link-arg=/STACK:8000000

cargo install --force scte35dump

@dholroyd Thanks for the suggestion. Unfortunately, it's the same result. I uninstalled scte35dump, ran the rust flag command and did the force install and I ended up with the same output.

I tried switching from clap to another cli arg parser, but I still get a crash when run in a windows VM using default compilation options.
Maybe you could try my previous suggestion again, but further increasing the default stack size for the resulting exe to 16MB,
set RUSTFLAGS=-C link-arg=/STACK:16000000

@dholroyd Sorry, reinstalling with that stack size didn't change the output. I should note that I'm not trying to do this in my own program. I'm only doing this in the CLI. I'm ignorant of how Rust works, so I don't know if that makes a difference.

Please could you try installing the latest code directly from github,
cargo install --force --git https://github.com/m2amedia/scte35dump.git

I made a small change that I hope may address the issue.

There is now a new 0.1.8 release that contains this fix plus a couple of updates to newer dependency versions.


GWL timing table

Description of PR
Summary of changes and related issue
[What's changed in this PR?]
Relevant motivation and context
[Why did you change this and what applicable context is needed to understand why this change is needed?]
Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[X] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Definition of Done Checklist
Practical

[ ] 80% unit test coverage
[ ] Documentation

[ ] All functions/adjusted functions documented in the readthedocs.
[ ] Documentation is pushed

[X] Complex code commented
[X] Naming conventions followed

[ ] Helper functions hidden with _ before the name

[X] Context of function is clearly provided

[X] Intent of function is provided
[ ] How to test, so that it is not siloed on scientists and anyone can review
[X] Appropriate manual testing was completed

[ ] Any notebooks known to utilize the affected functions are still working
[X] Linting completed and resolved

Conceptual

[X] Doesn't replicate existing functionality
[X] Aligns with general coding standard of existing functions
[X] Matches desired functinonality from users/scientists

 GWL timing table

This looks great @wkrantz. My one general comment is related to general consistency across AE as a whole. Here we have 1.5, 2.0, 2.5, and 3.0 as WL options. Our current WL code has 1.5, 2.0, 3.0, and 4.0. I am slightly inclined to include 2.5 as a user-facing option in our current WL code as we know it's going to be prominent for users, and for us to include 4.0 in this table.
Thoughts @nicolejkeeney?

@vicford Good point. Looks like @wkrantz added 2.5 since your comment! Thanks! :D


Separated the snapshot summary creation from the printing to improve testability.

I noticed that the coverage for the jest-cli reporters was not very high so I decided I could add some value by writing some tests for it:)
I split the creation of the snapshot summary to a separate file (get_snapshot_summary.js).
I wanted to write a test for the snapshot summary text printed in the console at the end of a text. However, the summary_reporter.js in which the snapshot summary creation is contained requires a lot of overhead to set up making focused testing difficult. This is why I split the creation of the snapshot summary from the reporting about it.
I am hoping that test coverage of the reporters can be gradually improved by splitting them up into smaller units with well defined and focused responsibilities that are easily testable.
Let me know what you think!

Codecov Report

Merging #4373 into master will increase coverage by 0.19%.
The diff coverage is 91.3%.

@@            Coverage Diff             @@
##           master    #4373      +/-   ##
==========================================
+ Coverage   55.85%   56.04%   +0.19%
==========================================
  Files         189      190       +1
  Lines        6383     6388       +5
  Branches        6        6
==========================================
+ Hits         3565     3580      +15
+ Misses       2815     2805      -10
  Partials        3        3

Impacted Files
Coverage Δ

...ackages/jest-cli/src/reporters/summary_reporter.js
12% <0%> (-7.7%)
:arrow_down:

...ges/jest-cli/src/reporters/get_snapshot_summary.js
100% <100%> (ø)

packages/jest-cli/src/reporters/utils.js
63.54% <0%> (+1.04%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4278b3...ce09cba. Read the comment docs.

Woohoo, more code coverage :) Thanks again @excitement-engineer. Looks good, nice diff.


Implement diagnostic EDMFX

[x] #1655
[x] #1724
[ ] #1722

Actually maybe we can close this? I will open smaller issues as we develop the diagnostic edmf.


[SMALLFIX] Update indents for CollectInfo.java

Update indents.

@jiacheliu3 PR reviewer.

Codecov Report

Merging #13800 (ce018b0) into master (0203366) will decrease coverage by 26.33%.
The diff coverage is 32.52%.

@@              Coverage Diff              @@
##             master   #13800       +/-   ##
=============================================
- Coverage     43.86%   17.52%   -26.34%
+ Complexity     8930     2674     -6256
=============================================
  Files          1359     1386       +27
  Lines         78109    80549     +2440
  Branches       9496     9803      +307
=============================================
- Hits          34260    14117    -20143
- Misses        40960    65184    +24224
+ Partials       2889     1248     -1641

Impacted Files
Coverage Δ

...n/java/alluxio/client/block/AlluxioBlockStore.java
5.93% <0.00%> (-69.91%)
:arrow_down:

...o/client/block/policy/DeterministicHashPolicy.java
0.00% <ø> (-68.43%)
:arrow_down:

...nt/block/policy/LocalFirstAvoidEvictionPolicy.java
0.00% <ø> (-94.29%)
:arrow_down:

.../alluxio/client/block/policy/LocalFirstPolicy.java
0.00% <ø> (-93.94%)
:arrow_down:

.../client/block/policy/MostAvailableFirstPolicy.java
0.00% <ø> (-91.67%)
:arrow_down:

.../alluxio/client/block/policy/RoundRobinPolicy.java
0.00% <ø> (-97.92%)
:arrow_down:

...lluxio/client/block/policy/SpecificHostPolicy.java
0.00% <ø> (-40.00%)
:arrow_down:

...ava/alluxio/client/block/stream/BlockInStream.java
0.00% <0.00%> (-58.20%)
:arrow_down:

...xio/client/block/stream/BlockWorkerDataReader.java
0.00% <0.00%> (ø)

...ient/block/stream/BufferCachingGrpcDataReader.java
0.00% <0.00%> (-41.67%)
:arrow_down:

... and 893 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 183552b...ce018b0. Read the comment docs.

alluxio-bot, merge this please


Update: Surveys and Observations

Updates to:

Survey Type
Survey Method
Geological Observation Type
Geological Observation Method

closes #206
closes #188
As always these are intended to be incremental change.  Further additions and augmentations to be added via future pull requests, during the development and UAT phase.
Please review as per roles below (you may have multiple roles).
This will count as approved with any approval combination that provides 2 SMEs, BOTH owners, and 1 technical review.
Reviewers
SMEs : @CantRoger @Greenwoodmatthew @LukeHauck
Vocab Owners: @Greenwoodmatthew (Surveys), @geoderekh (Observations)
Technical Review: @LukeHauck, @nicholascar, or @DavidCrosswellGSQ
FYI - Vocabulary Changes @lisa-woody

Tech is good.

Would like a couple of changes in relation to remote sensing surveys/observations, and one missed notation in surv_type in an otherwise excellent commit.

notation added
See discussion re collections: misattribution of concepts and omission of one collection


Refactor LibCurveDollarIncentive.sol _getPercentDeviationFromUnderPeg(uint256 amount)

Unreachable code was removed. It should be re-added and it should pass coverage CI.

The line was commented out because it was doing anything, and it's not doing anything as per original repo as this is unreachable code there

It's seem a simple solution the pavlovcik suggetion, but it gives the contributor (me) bad coverage, again a refactor might be need it

Originally posted by @molecula451 in https://github.com/ubiquity/ubiquity-dollar/issues/704#issuecomment-1606284776

looks like purged after all https://github.com/ubiquity/ubiquity-dollar/commit/3966447d8b5d7eba7b5f43ec2473d23d8b84b650

I thought the same, twap it's called many times over the whole file


Show how to match other file extensions

The readme is lacking in that it talks about enabling other file extensions, but only shows how to keep the default. For people not used to regex it could be confusing how they can enable other extensions.
Resolves #163

/\.mdx?$/ is not the default actually. The default is /\.mdx$/.
.mdx? means .md with optionally x appended. But this is a lot clearer.

Ah, that's good to know. Does that mean the ? should be removed from /\.(md|mdx)?$/?

Oh, missed that, yes it should be removed there otherwise it matches files ending with . too, not that that's common 😄

Fixed in #341


chore: enfore all review comments are resolved before merging

Signed-off-by: Aylei<EMAIL_ADDRESS>What type of PR is this?

[ ] API-change
[ ] BUG
[ ] Improvement
[ ] Documentation
[ ] Feature
[x] Test and CI
[ ] Code Refactoring

Which issue(s) this PR fixes:
As titled.

/merge


allow me to do everything at https://localhost:8443

When I go to https://<IP_ADDRESS>:8443 it says "In the web based Terminal experience, you can expect new features like: Visually focused loop experience with Autoloop built-in." and "Easily monitor routing activity and manage your channels through the dashboard.". If I'm already connected on localhost, why do I need to go to a third party website to do this? For one thing, it seems confusing to segregate features for no apparent reason. Also, it seems like a privacy invasion, even if you say at https://terminal.lightning.engineering/connect/pair/ that Lightning Labs can only see the size of data packets and if the node is connected. Seems like you can also see where my node is, when I use lightning terminal, etc..
Lightning Node Connect is a cool feature, but I don't know why it's required to see additional features.

Hi Andy, thanks so much for the feedback! As always, it's greatly appreciated. This feedback is especially helpful as we're in the middle of some conversations about how to move forward with the self-hosted Terminal UX. I'll go into detail below about why the current setup exists, but in case you're not interested in the full explainer, I'll provide a quick TLDR.
TLDR: We're actively exploring backporting some or most of the functionality of the web-based version of Terminal to the self-hosted version. It's a non-trivial amount of work, but we're leaning towards that path, mostly based on feedback like yours.
The relevant history is that after countless user interviews and feedback from node operators, we moved forward with a web-based solution for node management in an attempt to make managing a node more accessible, simple, and straightforward. Web development also allowed for us to move a bit quicker and give everyone the latest and greatest at the same time. terminal.lightning.engineering is a UI interface that LL quickly improves (without requiring users update their litd binaries) which communicates with user's machines running litd. If a user doesn't have an internet connection ( to access the terminal.lightning.engineering UI ) or doesn't want end-to-end encrypted traffic pass through LL's servers to their own node, they have the option to use the HTTP server in litd which we haven't been actively updating (but as mentioned above, that could change soon). It's always been a delicate balance of extending features for node's self-sovereign operational mode but delivering a UX that's on par with traditional web2 experiences.
In summary, historically, it was not trivial to keep both experiences updated with a bunch of new features. It was more of a resourcing question for us along with a strategic question whereby most users felt like a web-based experience would be more useful for their needs. However, as of late, we've heard more feedback around this and we feel like we have a path forward on a backport that wouldn't require a huge amount of resourcing so keep an eye out as we'll likely begin work on that this summer.

Actually, this may be a duplicate of https://github.com/lightninglabs/lightning-terminal/issues/556 .
Generally, I understand your desire to rapidly evolve the GUI on a hosted server. That service has some value for people who don't have a static IP address, but I don't see why we need a hosted server to have the features and it's also just really confusing to people (especially people who are setting up their node for the first time), especially if they only want to manage their lightning node from home. If you want to provide features first on your hosted server (for QA testing and also as a perk of using your hosted services), that might make sense, but I would only do that for a few weeks until they are proven and then push the changes to litd so everyone can use them. Right now it appears as though you aren't just behind pushing changes to litd, it seems like you have two completely different applications.
If you provide people an option to opt out of your hosted services, they will trust you more than if you make it mandatory.

Another thing that makes it really confusing is when you initially go to https://localhost:8443, the upper 50% of the screen implies that you must use the hosted server, then the lower 50% you are confused with something better you might get out of using the hosted server but because the sidebar on the left is collapsed you don't even know where to find those basic features you can currently get on https://localhost:8443 without poking around on every button until you figure it out.

Yes, it is a little bit of a duplicate of #556 and I pulled some of the responses from there into my initial response here. To be clear, it's not that we necessarily need the web architecture for certain features. It's just, in our initial user interviews in starting to push to resource the Terminal project, most of our interviewees indicated a strong desire for something like the web experience. Then, the practical reality is that we do have limited resourcing for the Terminal team overall. So, we made an initial decision to prioritize web given the gains it gave us from a speed, iteration, and consistency perspective (consistency here meaning being able to ship new features on top of older binaries rather than requiring binary updates to get the latest features).
Given this feedback and #556, which is good feedback and accurate, I do think we're clearly reaching a point where there's a strong desire for the feature set that exists on web in the locally hosted version. And, like you mentioned, a more consistent experience across both mediums would be best. As a thought exercise, IMO, if we started with split resourcing on web/local version with continuous updates for the latter, then we would've lost a good chunk of development time and I don't know if we would have been able to iterate with user feedback as quickly. Thus, we may not have even gotten to the point we are now where there is this pull demand for that feature set in the locally hosted version. So, I view it as an exciting opportunity that people really want those web features and are pushing us to backport those into the local version.
All that said, given this feedback, we have been actively exploring a path forward for a backport of the web feature set to the locally hosted version. But, not just a backport, our current favorite path forward would create an architecture that would allow for continuous updates on both user experiences in parallel. As you suggested, the local version would still lag behind slightly, but mostly due to the binary release/update cycle. We're prioritizing this path forward over the summer and should have something to address this in the fall. Again, really appreciate the feedback and thoughts here as it definitely helps us prioritize resourcing for the path forward.
For your second comment above, I see what you're saying and it wasn't our intent to signal that you must use the web version. I think we can improve the copy to make it more clear that some subset of the features still exists on the local version.

All your points sound fairly valid from a development perspective.
So, one thing you may want to choose a different name besides "web version". To me, the local instance can be exposed on the web too, so that gets confusing. Might be better to call it the hosted version.
One thing to note: I've never actually tried your hosted version so I don't even know what I'm missing (well, I might have popped it up for a minute when I was exploring someone else's umbrel that they just installed that had 0 channels setup yet). I like the idea of using swap services that your company may offer like loop and pool, but I have confidence in using those services because I can locally audit the software (choosing when I upgrade that software) to know the terms that we are doing business under. It's my understanding that your hosted service is providing software updates on the fly, but it is then doing local processing of encrypted data it gets from the node (through LNC). Since you have the ability to swap out that software at any time on me through the browser in your hosted version, it doesn't honestly make a difference to be whether the data is encrypted or not in transit, I still don't feel comfortable using such a service. Everybody has different needs though, so that's why I think a lot of people may prefer the hosted service instead.


Feedback: spam in feedback module

It looks like it's possible to post content there somehow without authorization
https://www.boonex.com/forums/topic/Deleting-Spam-Feedback-manually-.htm
https://www.boonex.com/forums/topic/Help-Too-many-Automatic-Spam-Report-.htm

if I update this file will this issue be resolved, my site is  getting hit with spam hard!

@tinboye Yes, but it's better to wait for official upgrade patch


[enhancement] Replace nose with nose2 #80

Migrated tests to nose2
Closes #80

Review Needed :smile:


TableRegion should become ComposedBlock

https://github.com/kba/page-to-alto/blob/46a8cc2fb74ce327e9d195f1095699cbae946cce/ocrd_page_to_alto/convert.py#L158
I think it's not enough to just map the lower levels here. There might not be any cell segmentation yet, only a detected table. And even if there is structure below that level, it's worthwhile mapping the recursive structure 1:1.
For that, there's the equivalent ComposedBlock in ALTO.

It is a ComposedBlock:
https://github.com/kba/page-to-alto/blob/46a8cc2fb74ce327e9d195f1095699cbae946cce/ocrd_page_to_alto/convert.py#L25
Since you're working with invoices and such, can you please share some samples for tables in PAGE-XML, then I can improve and test the table conversion.

It is a ComposedBlock:

Sorry, I was reading too sloppily.

Since you're working with invoices and such, can you please share some samples for tables in PAGE-XML, then I can improve and test the table conversion.

Sure. How about assets/data/gutachten/data?

For the sample gutachten/data/TEMP1/PAGE_TEMP1.xml, the current behavior seems to be correct:
<TableRegion>
  <TextRegion>
     <TextLine>

in PAGE becomes in ALTO:
<ComposedBlock>
  <TextBlock>
    <Textline>

I couldn't find a sample for a more complex table with deeper recursion than 1.

f138114 should support arbitrarily deep nesting in tables if I got the recursion right.

f138114 should support arbitrarily deep nesting in tables if I got the recursion right.

Yes, I think you did. But there are more cases: in PAGE, TextRegion can itself contain both nested TextRegions and immediate TextLines. And all region types are recursive, not just tables.
The problem is that in ALTO, TextBlock is not recursive, only ComposedBlock is. And ComposedBock is not allowed to have TextLines directly.
So you could (/probably need to) generalize the current pattern. But we would need to split up PAGE's "typed recursion" into ALTO's "pure recursion".
For example, if you have a GraphicRegion with embedded TextRegions, that would need to become a ComposedBlock comprised of an equally located/sized Illustration (which also maps its @type) followed by a list of TextBlocks for each embedded region.
Or if you have a TextRegion with immediate TextLines as well as embedded TextRegions, that would need to become a ComposedBlock comprised of an equally located TextBlock (with all the TextLines and its @type and @primaryLanguage), followed by a list of TextBlocks for the embedded regions.
Its unclear though, what to do with the TextEquiv at the region level (esp. if there's no line level below it) and other PAGE-specific info under TextRegion (like @leading / @align / @indented or @primaryScript or the order/direction attributes).

I'll try to implement basic and mixed-lines/regions recursion with ComposedBlock.

Its unclear though, what to do with the TextEquiv at the region level

There is nothing we can do I think. ALTO only allows content for String.
@leading could be mapped to @LINESPACE, @align is implemented via ParagraphStyle. @indented could be mapped to either @LEFT or @FIRSTLINE?

The behavior is buggy, it duplicates TextRegions within TableRegions in PAGE to a ComposedBlock and a TextBlock on the same level.


[Feature Request] Update function on existing structures

I don't know how you implement an update on an existing structures guys, but i think we can all agree that an update function for an existing structure would be awesome.
For now, it's fetch, copy, delete in db, edit copied, save for me.
how do you guys do updates on existing structures?

You don't need to delete in db to update your structure.

type User struct {
  ID int
  Name string
  Age int
}

db.Save(&User{
  ID: 1,
  Name: "John",
  Age: 10,
})

...

// now if you want to update the age field
// First fetch your user
var u User
db.One("ID", 1, &u)

// then update the field
u.Age = 20

// then save
db.Save(&u)

Of course, a function that would avoid fetching the object first would be great

I think  a update function for one field in a structure will be great, now I must pass all info with a structure is not so good.

To update, we need:

an instance of the type, so we can decode the record in it
the bucket name
the ID
the fields to update
the new values to set

So I was thinking about something like this:
err := db.Update(&User{ID: 1, Age: 20})

Basically, you pass an object with the values that need to change (plus the ID).
The good side is that you can update several fields at a time.
The downside is that if you want to set a field to a zero value you can't.
So we can add another function
err := db.UpdateField(&User{ID: 1}, "Age", 0)

You can only update one field
But you can set it to a zero value
@hardcoar @Comdex what do you guys think?

@asdine  I think so


functions to calculate a vectors magnitude and angle

Is there any functions for calculating a vectors magnitude and angle in this library?
I tried using polar() but the results are not what I expected. The following code prints 0.7375122532248497, but it should be 1.41... :
extern crate fpa;

use fpa::I3F29;

fn main() {
    let x = I3F29(1_i32).unwrap();
    let y = I3F29(1_i32).unwrap();
    let (a, _) = x.polar(y);
    println!("{}", a);
}

I've also tried different values, but I never get a value that would be the magnitude of the vector.
Thanks for this library!

That may be a convergence bug. 3.polar(4) does return 5.

Yeah, seems like this happens when both numbers are too close to each other. Thanks, guess I'll just ignore when this causes bugs, shouldn't be too often after all.


Hover tool over Image does not show values

When using Hover tool over Image, the values of the image intensity does not show up.
img = hv.Image(Intensity).options(tools = ['hover'], colorbar = True)

Could you try updating to holoviews 1.10.5?

I got this error: "AttributeError: type object 'Version' has no attribute 'setup_version" when I try to do: pip install git+https://github.com/ioam/holoviews.git.

Try updating param first with pip install param.

Just curious -- is "pip install git+https" a common way you install packages, compared to just "pip install holoviews"?  Seems like that's a good way to get not-quite-ready stuff...

After updating param, I can install holoviews 1.10.5, still no output image in the notebook.
I normally just use conda or pip install holoviews. In this case, I wanted to try the not-quite-ready stuff. I am not sure if they are different.

still no output image in the notebook.

That's extremely odd, is Intensity just a simple numpy array?

I'm afraid I'll need more detail, usually a fully reproducible example is preferred. Are you using the classic notebook or JupyterLab? Have you loaded the holoviews extension?

I used the notebook, not JupyterLab.  I am using 1.9.5 now, everything displays. I will keep using 1.9.5 for now to be little for proficient in holoviews. I guess I'll wait for the stable version.

I guess I'll wait for the stable version.

1.10.5 should be a lot more stable than 1.9.5. It really would be good to work out what's going on here. Could you simply paste the full code you're trying?

import numpy as np
import holoviews as hv
hv.extension('bokeh', 'matplotlib')

Bmode = np.random.randint(1, 200, (100,100))

img = hv.Image(np.flipud(Bmode), kdims = ['Lateral distance (mm)','Axial distance (mm)']).options(tools = ['hover'], colorbar = True, invert_yaxis = True)

%%output size=200
img

and the output of the cell is only this:
:Image   [Lateral distance (mm),Axial distance (mm)]   (z)

I figure out the problem: I needed to update ipython. I was using 5.x , I updated to 6.4. The image shows in the output cell, but the hover tool doesn't work.

@trongnguyen29 That's due to a bug in bokeh which causes the hover tool to break on a flipped yaxis (see https://github.com/bokeh/bokeh/issues/7959). Hopefully that can be fixed soon.

This has been fixed a while ago.


Extension.CurrentCredential()[Properties]

How to use Extension.CurrentCredential()[Properties] function inside FinishLogin function to add more parameters like resourceURL or datasourcepath

It can't. Extension.CurrentCredential() isn't valid inside FinishLogin and the properties aren't available yet -- they're actually determined by the result of FinishLogin.
The "FinishLogin" function can have two different signatures: either
(context as any, callbackUri as text, state as text) as record => …
or
(clientApplication as record, resourcePath as text, context as any, callbackUri as text, state as text) as record => …
If you want to pass an arbitrary serializable value from StartLogin to FinishLogin, you can have StartLogin return a Context field in the record in addition to the LoginUri, CallbackUri, etc.
However, if you just want the resourcePath you can declare the 5-argument version of the function instead of the 3-argument version of the function and just use the second argument.


Can kong support request_uri with args support matches in path?

Hi, there is an issue for us to deploy kong, replace the nginx also for api gateway support.
currently we use location in nginx "server" phase, with $request_uri which matches the specified args pair for route,
but when we transfer the route method with kong, seems we can only match the request with the path( req_uri without args), so I'm confused is there a better way to implement or something I don't know which support already?
I checked the contribution reference doc but not sure how the restriction style for feature request, can anyone do help? Thanks in advance.

Just checked the source code for router.lua, I guess the reason that not support request_uri with args support, because the cache mechanism for route match logic, that is really a good way to strip the args from request uri. But in our situation that is, we have to support the args matches. Just now I tried to re-write the router.lua, thinking if there is anyone to help me for a code review. thank you very much.
The main way I just added another function find_route_ng with the additional parameter "orig_req_uri" passed, and only used for regex matches with uri mode; also disallow the cache setting.
the only issue I currently know is, see below sample:

uri: /api/mineapp/res?foo=bar
expected path[]=/api/mineapp/res.*foo=bar route to service A

uri: /api/mineapp/res
expected path[]=/api/mineapp route to service B

while both of them be matched at once, the point 1 would be route to service B by the reason of the cache key are same.
router.lua.txt
see attached source code. BTW, the version of kong is: 1.4.0

@mnteam, you are correct, we don't have arbitrary query parameter based routing. Though we did add header based routing which is quite similar to this, e.g. key-value pair matching. In that sense I think it would we a welcomed addition. One way to workaround this is to write plugin that runs on rewrite and turns query parameters to headers (and removes them on access) and then use header based routing, but it is cumbersome.

@bungle Hi Bungle, thank you for your response, I already changed the router.lua(see previous attached txt file) to support this feature, did the test and seems it is ok, but the bad news is: because of the uri based cache key generation, I have to disable the cache support for route find.( I see the header matches also disallow the cache, but confused if they have there are two same request uri but different head value, one of them have no header value matches, it should be an issue which would cause the incorrect route mapping)
the next we will have a test scenario for the no cache route matches to see how it would be.

@mnteam great feedback! feel free to open a pull-request (or two if you think there is a bug in current header matching).

@bungle hi Bungle, seems I have no permission to create individual branch for pull request, and I did not see related reference about how to open a pull request with specified way, any hint? Thank you.
for head matching of route, I do not think there is a way to simply resolve the issue, so my solution is disable the cache for route matching.

@mnteam,

fork: https://github.com/Kong/kong/
create branch in your fork
open pr from your fork to https://github.com/Kong/kong/

@bungle  hi Bungle, sorry for the late reply, I just sent a pull request, please can you help to code review? the link is: https://github.com/Kong/kong/pull/5347.
Please inform me if there is any issue or I get the wrong understanding.

@mnteam thanks for pr, I am sure we will look at it soon, good stuff!


snapshot testing inverse regex example documentation

Issue:
What I did
Added documentation about using inverse regex to exclude stories from snapshot testing following this issue https://github.com/storybooks/storybook/issues/892#issuecomment-307537901
How to test

@tmeasday LGTM indeed and you are welcome. Thanks for all your work as well.


ValueError: path is on mount 'D:', start on mount 'F:'

HtmlTestRunner version: pip install
Python version: 3.7
Operating System: window

Description

Describe what you were trying to get done.
Tell us what happened, what went wrong, and what you expected to happen.
What I Did
Paste the command(s) you ran and the output.
If there was a crash, please include the traceback here.

I counld output report to another mount

@oldani

When I delete the os.path.relpath,everything is OK


Parse compiled cjs step definitions

🤔 What's changed?
Updated the javascript defineStepDefinitionQueries array to include a Tree-sitter query that matchs steps in the format found in compiled commonjs files.
Javascript steps in the following format will now be identified:
;(0, cucumber_import.Given)('a compiled format', function () {
  ...
})

NOTE: The function used for the second argument is completely ignored by the query - it makes no difference if it is async or not, the step will still be matched.
⚡ What's your motivation?
Fixes #187 - allows steps to be identified in compiled cjs/js files, usually needed when importing from other modules (aka in node_modules)
🏷 What kind of change is this?

:zap: New feature (non-breaking change which adds new behaviour)

♻ Anything particular you want feedback on?

Are the doc changes required/suitable?

📋 Checklist:

[x] I agree to respect and uphold the Cucumber Community Code of Conduct
[x] I've changed the behaviour of the code

[x] I have added/updated tests to cover my changes.

[ ] My change requires a change to the documentation (potentially, not crucial though)

[x] Users should know about my change

[x] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

Awesome contribution @finnmerlett! Extended the query to support direct imports with this step definition format also (i.e. Given rather than cucumber.Given)

Thanks for the merge 💪 good to see the fix make its way in! Just FYI in terms of the edit you made, doesn't do any harm but probably isn't needed, since in cjs compiled format where you have (0, import.property)(...) it is always a property access and never a direct import usage.
If you're interested, the reason (which I only found out when I was looking stuff up for this fix) that you find this weird syntax in compiled cjs files is because it is a quick way to ensure the this binding inside the property is set to the global scope (aka undefined in strict-mode). By returning the property accessor from an immediate expression evaluation aka (0, import.property), you detach it from its object without having to assign it to an intermediary variable.
The global-this-context'ed usage of member imports is what the esm import style import { method } from 'package' does automatically, but since commonjs uses require() this hack is the standard shorthand to mimic the behaviour.

@kieran-ryan

Awesome contribution @finnmerlett! Extended the query to support direct imports with this step definition format also (i.e. Given rather than cucumber.Given)

Thanks for the merge 💪 good to see the fix make its way in! Just FYI in terms of the edit you made, doesn't do any harm but probably isn't needed, since in cjs compiled format where you have (0, import.property)(...) it is always a property access and never a direct import usage.
If you're interested, the reason (which I only found out when I was looking stuff up for this fix) that you find this weird syntax in compiled cjs files is because it is a quick way to ensure the this binding inside the property is set to the global scope (aka undefined in strict-mode). By returning the property accessor from an immediate expression evaluation aka (0, import.property), you detach it from its object without having to assign it to an intermediary variable.
The global-this-context'ed usage of member imports is what the esm import style import { method } from 'package' does automatically, but since commonjs uses require() this hack is the standard shorthand to mimic the behaviour.


Auto open?

I might be misunderstanding something but I thought it was not necessary to open and wait for the db to open? However, I'm still getting a Database is not open error when doing something like:
const db = new ClassicLevel(...)
const data = db.getMany(...) /// kStatus === 'closed'

I'm guessing the database open fails and somehow the error doesn't nicely propagate to getMany?

const db = new ClassicLevel(...)
const data = db.getMany(...) /// kStatus === 'closed'

Can you share a complete example with arguments? You're right that it should not be necessary to wait for open, but your example doesn't await the result of getMany() (and therefore the result of opening) so I'm not sure how to interpret it.

I'll try to make a repro.


How can i pass image from uri at node.js

Couldn't find how to pass image to human.detect from uri, i prepared a function like in demo but doesn' accept as valid input;
let url = "https://firebasestorage.googleapis.com/v0/b/zetform-63ea7.appspot.com/o/organizations%2FHrJUmJjOAAgtmhiI4PK6%2Fmonitorings%2FGZoPjyFTjua0iWYQq2Ym%2Ftestinstances%2Fk1ULsrMNJU3ZUtk7GcJe%2Fphotos%2F1_2021-01-17%2010%3A15%3A48.jpg?alt=media&token=22024c06-a99e-4290-b59c-6a2da1b0786b";
    async function urlImage(url: string): Promise<Canvas> {
        const img = await canvas.loadImage(url);
        const c: Canvas = canvas.createCanvas(img.width, img.height);
        const ctx = c.getContext('2d');
        ctx.drawImage(img, 0, 0, img.width, img.height);
        return c;
    }

const tempCanvas : Canvas  = await urlImage(url);
const result: LX.vladHuman.Result = await human.detect( tempCanvas , myConfig) as LX.vladHuman.Result;

My real question is to load image from uri and pass to detect function in healty way, there is a good example in demo but it loads images from disc

Best.

fair question, i've just added example to demo/node.js: https://github.com/vladmandic/human/blob/main/demo/node.js#L51
it can be done using canvas library as well, i just like this as there are no external dependencies other than fetch

Sure it is solved thank you


remove unused nullAEAD struct in the quic package

Codecov Report

Merging #1446 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1446      +/-   ##
==========================================
+ Coverage   89.21%   89.25%   +0.04%
==========================================
  Files         121      121
  Lines        8472     8468       -4
==========================================
  Hits         7558     7558
+ Misses        615      611       -4
  Partials      299      299

Impacted Files
Coverage Δ

server_tls.go
78.15% <ø> (+2.02%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7f2c49...24c5023. Read the comment docs.


Add compile-time AVX support check to TrampolineTest

Compilation on a processor that doesn't support AVX fails otherwise.

@danielfenner , without the check, I get this on a personal machine running Ubuntu 20.04:
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1020:9: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
  CHECK(_mm256_extract_epi64(p0, 0) == 0 && _mm256_extract_epi64(p1, 0) == 0 &&
        ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1020:45: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
  CHECK(_mm256_extract_epi64(p0, 0) == 0 && _mm256_extract_epi64(p1, 0) == 0 &&
                                            ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1021:9: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p2, 0) == 0 && _mm256_extract_epi64(p3, 0) == 0 &&
        ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1021:45: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p2, 0) == 0 && _mm256_extract_epi64(p3, 0) == 0 &&
                                            ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1022:9: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p4, 0) == 0 && _mm256_extract_epi64(p5, 0) == 0 &&
        ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1022:45: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p4, 0) == 0 && _mm256_extract_epi64(p5, 0) == 0 &&
                                            ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1023:9: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p6, 0) == 0 && _mm256_extract_epi64(p7, 0) == 0);
        ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1023:45: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
        _mm256_extract_epi64(p6, 0) == 0 && _mm256_extract_epi64(p7, 0) == 0);
                                            ^
/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
  (long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1037:32: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
          CheckM256iParameters(_mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                               ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1037:55: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
          CheckM256iParameters(_mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                                                      ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1037:78: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
          CheckM256iParameters(_mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                                                                             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1038:32: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
                               _mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                               ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1038:55: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
                               _mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                                                      ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1038:78: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
                               _mm256_set1_epi64x(0), _mm256_set1_epi64x(0), _mm256_set1_epi64x(0),
                                                                             ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1039:32: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
                               _mm256_set1_epi64x(0), _mm256_set1_epi64x(0));
                               ^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1039:55: error: always_inline function '_mm256_set1_epi64x' requires target feature 'avx', but would be inlined into function 'TestBody' that is compiled without support for 'avx'
                               _mm256_set1_epi64x(0), _mm256_set1_epi64x(0));
                                                      ^
16 errors generated.
make[2]: *** [src/UserSpaceInstrumentation/CMakeFiles/UserSpaceInstrumentationTests.dir/build.make:232: src/UserSpaceInstrumentation/CMakeFiles/UserSpaceInstrumentationTests.dir/TrampolineTest.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:2722: src/UserSpaceInstrumentation/CMakeFiles/UserSpaceInstrumentationTests.dir/all] Error 2
make: *** [Makefile:95: all] Error 2
ERROR: conanfile.py (OrbitProfiler/None): Error in build() method, line 174
	cmake.build(target=str(self.options.build_target) if self.options.build_target else None)
	ConanException: Error 2 while executing cmake --build '/home/pierric/git/orbit/build_clang9_relwithdebinfo' '--' '-j4'

/usr/lib/llvm-9/lib/clang/9.0.1/include/avxintrin.h:2024:14: note: expanded from macro '_mm256_extract_epi64'
(long long)__builtin_ia32_vec_ext_v4di((__v4di)(__m256i)(X), (int)(N))
^
/home/pierric/git/orbit/src/UserSpaceInstrumentation/TrampolineTest.cpp:1022:45: error: '__builtin_ia32_vec_ext_v4di' needs target feature avx
_mm256_extract_epi64(p4, 0) == 0 && _mm256_extract_epi64(p5, 0) == 0 &&

Looks like  the intrinsics get resolved to these "_builtin_ia32*"s and then the instructions cannot be emmitted since AVX is turned off. My understanding is that this should not happen since you are using build_clang9_relwithdebinfo profile and this transitively includes linux_common. which sets -march=sandybridge .
Also my understanding is that it should not matter which instructions the cpu running the compiler supports. This should either happen on all systems or on none.
Obviously things do not work as described above. Let's chat offline and see if we can fix this.

@danielfenner , as you pointed out, this should work fine as we can compile those instructions even if a machine doesn't support AVX. Wiping my build_* directories and recompiling fixed the issue. Nothing to see here. Closing the PR.


Clean up task service implementation

This is some clean up around the address piece for task services. The goal here was to keep consistent with our already established pattern which maybe wasn't as obvious as it could be so I'm putting it here:
request JSON from server -> validate JSON (throw if invalid) -> convert TS type representing valid JSON to TS type representing valid store object
In this particular scenario each address from the server comes in the form: {address_type: string, address: object} once validated our TS type: ValidatedAddressWithTypeJSON is then converted to its store representation: Address which has its type field embedded.

Holding off on this PR until we can talk in person

My reasonings about the changes I introduced:
-AddressWithType was unnecessary and added no real value. The only reference to AddressWithType was from the AddressBuilder which was never used.

Putting the type right on the Address makes filtering addresses simple
Using addresses on Service keeps our mapping between JSON type and store type simpler (see serviceFromValidatedJSON) The postalAddress was unused so it was removed.
Previously Address was being used to represent both our JSON address and our store address but actually only reflected the JSON version so I've renamed it to ValidatedAddressJSON and the created Address for our store type.

My thinking for handling addresses and phone numbers differently is that there are a million different phone number types, but only two address types that I know of. However, I'm not going to insist on that, there also value to be consistent between phone numbers and addresses.
Generally, there can be value to having both AddressWithType and Address. When the store address type as postal_address and physical_address, we need an Address  type without the type field on it, since it would be redundant and we could get bugs where the postal_address could well end up with a type of physical_address, ugh. However, the json parser would output an address with the type field on it, so we'd need both. But if we go with the same approach as phone numbers, then we need the type field. We end up with more business logic in the presentation to pick out the address, where you can get bugs like not just having a missing physical address, but you could have multiple addresses with that type. (My reasoning around what data structure to use is really to find a solution where invalid situations are impossible to represent).

Can you update all commit messages to reference #259?

@rasmus-storjohann-PG Cheers for the feedback. I do see your points as well. I've addressed the issues you've outlined.


[SPARK-39508][CORE][PYTHON] Support IPv6 between JVM and Python Daemon in PySpark

What changes were proposed in this pull request?
This PR aims to use IPv6 between Spark and Python Daemon in IPv6-only system.
Unlike spark-shell, pyspark starts Python shell and java-gateway first.
We need a new environment variable, SPARK_PREFER_IPV6=True in pyspark shell, like the following.
SPARK_PREFER_IPV6=True bin/pyspark --driver-java-options=-Djava.net.preferIPv6Addresses=true

Why are the changes needed?
Currently, PySpark uses <IP_ADDRESS> for inter-communication between Python Daemon and JVM.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass the CIs.

Rebased to the master in order to bring #36905 .

Hi, @HyukjinKwon . How do you think about this PR? This is a preparation for PySpark UTs.

Will review this today or tomorrow.

Merged to master.

Thank you so much, @HyukjinKwon !


Response type should support multiple messages

I am using a custom chat endpoint where the request is handled by a group of agents, and therefore, the response comes as a list of messages. It looks like DeepChat is only prepared to handle a single message in the Response: https://deepchat.dev/docs/connect/#Response
It would be great if this assumption can be relaxed.

I would also like this, but in case you haven't seen it, there's a workaround using the websocket handler.

@mchill Interesting but this is going to be tricky for me as my endpoint is served by ReactPy in FastAPI so I actually have no easy way to create Javascript objects. This is my UI endpoint in pure Python:
@component
def Agent():
    return DeepChat({
        "request": {"url": "http://localhost:8000/chat"}
    })

Hey folks, apologies for the late replies as I am currently very occupied with my current job.
I wanted to offer an alternative, you could potentially intercept the incoming responses using the responseInterceptor, parse the content, and use a method called _addMessage multiple times instead. This method is not part of our official documentation and will be added to the main API on our next release, however it is still available in the current release. The way it works is it accepts a Response object as an argument and adds a message to the chat. E.g.
deepChatRef._addMessage({text: 'hello'});
Let me know if this helps.
Thanks!

@OvidijusParsiunas There is a gotcha with responseInterceptor. It is still expected to return a valid Response object which ends up repeating one of the messages. Returning null from responseInterceptor is treated as an Error.

<html>
    <head>
        <script<EMAIL_ADDRESS>type="module" crossorigin></script>
    </head>
    <body>
        <deep-chat id="chat" demo="true" request='{"url": "/chat"}' stream='{"simulation": 6}'></deep-chat>
        <script>
            window.onload = function() {
                let chatElementRef = document.getElementById('chat');
                chatElementRef.responseInterceptor = (response) => {
                    console.log(response); // printed above
                    response.forEach(m => chatElementRef._addMessage(m))
                    return response[0];
                };
            };
        </script>
    </body>
</html>

I am now calling _addMessage for all messages except the last one and returning the last one from the responseInterceptor. This works:
[
  {
    "text": "Cathy, tell me a joke.",
    "role": "joe"
  },
  {
    "text": "Sure, here's one for you:\n\nWhy don't scientists trust atoms?\n\nBecause they make up everything!",
    "role": "cathy"
  }
]

            window.onload = function() {
                let chatElementRef = document.getElementById('chat');
                chatElementRef.responseInterceptor = (response) => {
                    console.log(JSON.stringify(response, null, 2));
                    response.slice(0, response.length - 1).forEach(m => chatElementRef._addMessage(m))
                    return response[response.length - 1];
                };
            };

Now there is just a minor glitch that the role from the last message is not respected.

Hi @nileshtrivedi.
It is strange that the returned role is not respected.
Could you perhaps make the last message returned by the interceptor define an explicit role?
E.g. return {role: 'your-role-name', text: response[response.length - 1].text}

@OvidijusParsiunas That does not fix it.
Here is the whole app in a single file. You can run this as OPENAI_API_KEY=api_key fastapi dev app.py:
import os
import json

import tempfile
from typing import Annotated, Literal
from pathlib import Path
from autogen import ConversableAgent, register_function
from autogen.coding import LocalCommandLineCodeExecutor

from fastapi import FastAPI, Body, Request, WebSocket
from fastapi.responses import HTMLResponse
from fastapi.staticfiles import StaticFiles

# from pydantic import BaseModel, List
# from prefect import flow

openai_api_key = os.environ.get("OPENAI_API_KEY")

cathy = ConversableAgent(
    "cathy",
    system_message="Your name is Cathy and you are a part of a duo of comedians.",
    llm_config={"config_list": [{"model": "gpt-4", "temperature": 0.9, "api_key": openai_api_key}]},
    human_input_mode="NEVER",  # Never ask for human input.
)

joe = ConversableAgent(
    "joe",
    system_message="Your name is Joe and you are a part of a duo of comedians.",
    llm_config={"config_list": [{"model": "gpt-4", "temperature": 0.7, "api_key": openai_api_key}]},
    human_input_mode="NEVER",  # Never ask for human input.
)

app = FastAPI()

@app.get("/", response_class=HTMLResponse)
async def root():
    homepage_html = """
<html>
    <head>
        <script<EMAIL_ADDRESS>type="module" crossorigin></script>
    </head>
    <body>
        <deep-chat id="chat" demo="true" request='{"url": "/chat"}' stream='{"simulation": 6}' names="true"></deep-chat>
        <script>
            window.onload = function() {
                let chatElementRef = document.getElementById('chat');
                chatElementRef.responseInterceptor = (response) => {
                    console.log(JSON.stringify(response, null, 2));
                    response.slice(0, response.length - 1).forEach(m => chatElementRef._addMessage(m))
                    console.log(response[response.length - 1])
                    return {role: "cathy", text: response[response.length - 1].text};
                };
            };
        </script>
    </body>
</html>
    """
    return homepage_html

@app.post("/chat")
async def chat(request: Request):
    data = await request.json()
    user_msg = data["messages"][0]["text"]

    result = joe.initiate_chat(cathy, message="Cathy, tell me a joke.", max_turns=1)
    return [{"text": m["content"], "role": ("joe" if m["role"] == "assistant" else "cathy")} for m in result.chat_history]

You can try this, it will correctly identify the identity. This is the document about this parameter.
You need to delete "stream={" simulation ": 6} ".
https://deepchat.dev/docs/messages/#names
<deep-chat id="chat" demo="true"  names='{"user":{"text":"cathy"}}'></deep-chat>
<script>
  window.onload = function() {
    let chatElementRef = document.getElementById('chat');
    chatElementRef.responseInterceptor = (response) => {
      response = [
        {
          "text": "Cathy, tell me a joke.",
          "role": "joe"
        },
        {
          "text": "Sure, here's one for you:\n\nWhy don't scientists trust atoms?\n\nBecause they make up everything!",
          "role": "user"
        }
      ]
      console.log(JSON.stringify(response, null, 2));
      response.slice(0, response.length - 1).forEach(m => chatElementRef._addMessage(m))
      console.log(response[response.length - 1])
      return response[response.length - 1];
    };
  };
</script>

@buzhou9 Yes, deleting "stream={" simulation ": 6} " indeed fixes the names. Thanks again to both you and @OvidijusParsiunas ! 👍


Use of stbtt_ScaleForPixelHeight Incorrect in typographer_context_stb.cc

Steps to reproduce

Run a test case which uses the stb based glyph atlas implementation.
Examine the contents of the Glyph Atlas at runtime using renderdoc or other

Expected results
The size of the glyphs in the atlas should be correct for a given typeface size in Points.
That is, for a typeface of size 12pt, the difference between the highest ascender and lowest descender should be 16 pixels (12pt == 16px @ 160 dpi).
Actual results
Currently, the resultant glyphs are (for a 12pt font) about 12px` high. This is despite correct scaling calculations.
See the screenshots below.
Code sample
Code sample
[Paste your code here]

Screenshots or Video

Screenshots / Video demonstration
Here's an analysis of the issue.
Baseline behavior for the Noto Sans typeface (normal, weight 400) can be gathered here.
This looks like:

Note that the vertical bar character (linux "pipe" symbol) covers the range from the highest ascender to the lowest descender. Thus it is 16px high for a 12pt font.
Looking at the current glyph atlas for an example using the same typeface at the same size (using renderdoc):

The vertical bar is 12 pixels high. This is plainly incorrect.
Debugging a little, we find the scaling calculations are correct. We scale the desired 12pt to 16px at runtime.
I've debugged this further and find that the function call stbtt_ScaleForPixelHeight is returning incorrect scale values for correct inputs.
Using the function stbtt_ScaleForMappingEmToPixels with identical inputs results in correctly sized glyphs.
I'll try to present a PR to change this function call ASAP.
CC @bdero and @chinmaygarde

Logs
Logs
[Paste your logs here]

Flutter Doctor output
Doctor output
[Paste your output here]

Here's a similar issue in stb.
They recommend moving from stbtt_ScaleForPixelHeight to stbtt_ScaleForMappingEmToPixels and that seems to work in my tests.
I'm going to put up a PR making that change and we can see if it's acceptable.


Error when accessing Oasis Web wallet

Http response at 400 or 500 level
The above error occurred in task ft
created by takeEvery(network/selectNetwork, ft)
created by ht
Tasks cancelled due to error:
takeEvery(network/selectNetwork, ft)
D@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1047878
Ft@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1070696
Ut/<@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1069926
Ue@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1056709
Be@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1056515
xt@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1064004
Tt@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1064785
r.W@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1065728
r.R@https://wallet.oasisprotocol.org/static/js/2.48cea8b1.chunk.js:2:1065705

Cross referencing #428, where I'm also asking if we can restore a stack trace like this

Thank you for reporting this issue. We have updated our Oasis Wallet with bug fixes and new features which should also solve mentioned issue above.
If you still encounter same or similar issue, please check our FAQ or send an email to our support team at<EMAIL_ADDRESS>


Support multiple Spring version integration tests

Description
Support multiple Spring version integration tests based on configuration file spring-cloud-azure-supported-spring.json.
All SDK Contribution checklist:

[x] The pull request does not introduce [breaking changes]
[ ] CHANGELOG is updated for new features, bug fixes or other significant changes.
[x] I have read the contribution guidelines.

General Guidelines and Best Practices

[x] Title of the pull request is clear and informative.
[x] There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

[ ] Pull request includes test coverage for the included changes.

/azp run java - spring - tests

/azp run java - spring - tests

/azp run java - spring - tests

/azp run java - spring - tests

Hi @JimSuplizio, @benbp, @hallipr, @weshaggard, can any of you please review this PR? Thanks in advance!

Hi @JimSuplizio, @benbp, @hallipr, can any of you please review this PR? Thanks in advance!

@moarychan, I'm adding @alzimmermsft in case he has any feedback. I am going to say that it's kind of hard to review a PR considering the state of the java - spring - tests pipeline results are indicative of something catastrophic going on. I cannot approve these changes at this time.

API change check
API changes are not detected in this pull request.

Now Spring tests run almost completely successfully, I will re-run the failed one, @JimSuplizio and @alzimmermsft, please help review. Many thanks!

I will review this today

/check-enforcer override


Please make option to use MPS (M chips by Apple)

Search before asking

[X] I have searched the Autodistill issues and found no similar feature requests.

Description
As I know many libraries now support Apple chips which means we can use them for training and inference. Please make it possible to use because now I'm able use only CPU which is very slow.
Use case
No response
Additional
No response
Are you willing to submit a PR?

[ ] Yes I'd like to help by submitting a PR!

You can do this by using pip install onnxruntime-silicon after installing inference. Note that this won’t work in Docker because Apple doesn’t expose the MPS device to Docker yet.

You can do this by using pip install onnxruntime-silicon after installing inference. Note that this won’t work in Docker because Apple doesn’t expose the MPS device to Docker yet.

So it will automatically put any process from the module to mps(gpu)?
Thank you for the help 🙏

Honestly, I was trying to understand how I can integrate ONNX to my code, but I boiled my brain instead :)
Import necessary libraries
from autodistill_grounded_sam import GroundedSAM
from autodistill.detection import CaptionOntology
from autodistill.utils import plot
import torch.onnx
import cv2
import torch
Define an ontology to map class names to our GroundingDINO prompt
ontology = CaptionOntology({"boxing glove": "boxing glove"})
Load the model
base_model = GroundedSAM(ontology=ontology)
base_model.label("./data", extension=".png")
Please let me know what I should do


Document how to deal with GitHub disabling CompatHelper workflows after 60 days of inactivity

GitHub disables actions after 60 days of inactivity and there is discourse about how to address it but I cannot quite tell what it the appropriate solution:
https://discourse.julialang.org/t/compathelper-workflows-disabled-after-60-days-of-inactivity/48870
It would be great if the README here would address this issue specifically and definitively because an Oct. 2020 comment there suggests that a CompatHelper action will be "effectively deprecate[d]" but I still see lots of actively maintained packages using it so I doubt that prediction has come true yet
https://discourse.julialang.org/t/compathelper-workflows-disabled-after-60-days-of-inactivity/48870/9
Should we package developers be doing anything other than using the latest CompatHelper.yml file?

Just commented on this.
We've created an AWS Lambda solution for this problem, KeepActionsAlive, which automatically re-enables your workflows.


vendor: fixing bad state

if folks are concerned about this i can dig in deeper, but my and matt's explorations with zipkin put my deps in a weird state, so here's me fixing them

nvm i'm a goof.


Issues with bulding OpenRLHF locally

Hi,
building the project in a python venv using setup.py has the following issues.

wheel and packaging are missing from requirements.txt required by flash-attn
Running train_ppo_llama.sh leads to flash_attn_2_cuda.cpython-310-x86_64-linux-gnu.so: undefined symbol: _ZN2at4_ops5zeros4c... error. The only way I could overcome this is by manually rebuilding flash-attn with FLASH_ATTENTION_FORCE_BUILD=TRUE pip install --force-reinstall flash-attn which takes a long time. Is there a better way to setup the project?

Thanks
Environment:
Ubuntu 20.04
Python 3.10
CUDA 12.1

We usually use docker to solve this problem, refer to our dockerfiles

Hi,
pip install packaging ninja
then, you can download specified version from https://github.com/Dao-AILab/flash-attention/releases.

https://github.com/OpenLLMAI/OpenRLHF/issues/251#issuecomment-2017132081


more robust workaround for -d command line params

unfortunately there were some corner cases (like first parameter not equal to -d) that are difficult to handle with sh. added a more robust workaround script (which requires bash) and made clear we have to set the PHP_BINARY env variable for this to work.

Thanks. The long term solution would be:

to automatically set the env var when running frankenphp php-cli (easy pick)
support -d (a bit more complicated, but basically some copy/paste/adapt from the official CLI SAPI to do)

Technically, this would be possible to execute the php-cli subcommand if no explicit command is passed and arguments exist, but I fear that it would be confusing and error prone.
Creating a symlink or a tiny script that calls frankenphp php-cli would be very similar isn't it?

With the wrapper script, the potential problem is for frankenphp to know whether it is being called in the context of a wrapper script / where the wrapper script is. If I call frankenphp php-cli composer install directly on the terminal, is there a good way for frankenphp to know where the wrapper script could be?
Alternatively, one could make frankenphp behaved differently if it was called "php" rather than anything else (or called through a symlink named "php"). I was able to get this work on linux and mac, windows not tested (invokedAs := filepath.Base(os.Args[0])). So effectively frankenphp would bundle its own wrapper script. However, this behavior is also a little bit subtle.

I'm a bit reluctant do add such hacks in FrankenPHP itself. Wouldn't it be more reliable and easier to add support for php-cli to Composer and Artisan directly?

For frankenphp to be mostly compatible with the existing command line arguments of php-cli (i.e. run php -d memory_limit=1G myscript.php through frankenphp) is really important for a smooth migration path in my view. Convincing other projects to add explicit support for frankenphp's existing CLI interface may be possible, but will delay and limit adoption of frankenphp (e.g. if things go well, laravel 12 may include frankenphp patches; if your legacy application has shell scripts calling php, you have to update them all, what about semi-proprietary stuff like Magento/Adobe Commerce)
The good news is that the command line interfaces of caddy/frankenphp and "classic" php do not conflict and hacks like os.Args[0] are not required; each user can decide whether to symlink frankenphp to php or not.
Suggested Change in Behavior

If we receive command line arguments but no valid frankenphp/caddy command, default to php-cli
In case of an error when loading the php script after defaulting to php-cli print a warning in addition to the error message
Add support for at least the flags -d, -i and -m to improve support with legacy scripts

Examples of proposed behavior:
$ frankenphp -d memory_limit=1G artisan
Laravel Framework 10.39.0

Usage:
...

$ frankenphp artisan queue:failed
   Illuminate\Database\QueryException

  SQLSTATE[08006] [7] connection to server at "<IP_ADDRESS>", port 5432 failed: Connection refused
...

$ frankenphp start-servers; echo $?
Warning: unknown command "start-servers"; defaulting to "php-cli"
Could not open input file: start-servers
1

Comparison with existing behavior:
$ frankenphp start-servers; echo $?
Error: unknown command "start-servers" for "caddy"
1

What do you think about this solution? if you could consider a PR along these lines, I'd be happy to give it a try.

@flexponsive let's merge this one for now.
Supporting -d would be cool and should be easy.
I'm less sure about the "default" behavior as it will create ambiguities but I have to think more about this.

Thanks!


feat(web): npm workspaces & shared code

This PR makes use of NPM workspaces to arrange crux and crux-ui packages differently. This makes it possible to create a common package to shared code between them.

Talked with @m8vago, we are not happy with the solution.. trying other things instead.


Map View Fails to Center on User's Location

Summary
I am experiencing an issue with react-native-maps where the map fails to center on the user's current location after relaunching the app.
When I open the app for the first time, the map correctly displays and centers on the user's current location. However, after closing and reopening the app, the map view does not center correctly. Instead, it positions the user's location at the (0, 0) x, y point of the map view, leaving the location off-center.
I attempted to recenter the map using the animateToRegion function, but it still fails to center the user's location in the map view. However, if I manually drag the map slightly and then call animateToRegion, the centering works correctly.
Reproducible sample code
import React, { useEffect, useRef } from 'react';
import MapView from 'react-native-maps';
import Geolocation from '@react-native-community/geolocation';

const App = () => {
  const mapRef = useRef(null);

  useEffect(() => {
    Geolocation.getCurrentPosition(
      (position) => {
        const { latitude, longitude } = position.coords;
        const region = {
          latitude,
          longitude,
          latitudeDelta: 0.01,
          longitudeDelta: 0.01,
        };
        mapRef.current?.animateToRegion(region, 1000);
      },
      (error) => console.log(error),
      { enableHighAccuracy: true, timeout: 15000, maximumAge: 10000 }
    );
  }, []);

  return (
    <MapView
      ref={mapRef}
      style={{ flex: 1 }}
      initialRegion={{
        latitude: 37.78825,
        longitude: -122.4324,
        latitudeDelta: 0.01,
        longitudeDelta: 0.01,
      }}
    />
  );
};

export default App;

Steps to reproduce

Launch the app for the first time. (The map correctly centers on the user's current location.)
Close and relaunch the app. (Observe that the user's location is displayed, but it is positioned at the (0, 0) x, y point of the map view instead of being centered.)
Call animateToRegion; the user's location remains off-center.
Drag the map slightly and call animateToRegion again. (The map now centers correctly on the user's current location.)

Expected result
The user's location should always be centered in the map view, both when the app is opened for the first time and after relaunching.
Actual result
After relaunching the app, the user's location is shown but is positioned at the (0, 0) x, y point of the map view instead of being centered. The animateToRegion function does not resolve the issue unless the map is manually interacted with first.
React Native Maps Version
1.20.0
What platforms are you seeing the problem on?
Android, iOS (Google Maps)
React Native Version
0.75.4
What version of Expo are you using?
Not using Expo
Device(s)
iPhone 15 Pro Max (17.6.1), iPhone 15 (17.6.1)
Additional information

This behavior occurs only after relaunching the app.
This issue primarily occurs on physical devices and in release builds. It is less likely to be observed in emulators or debug builds.
Location permissions are correctly configured.
Manually interacting with the map temporarily resolves the issue, allowing animateToRegion to function as expected.
I also managed the reproduce same issue on react-native-maps version 1.7.1.
Workaround: I managed the go desired location by following code:

mapRef.current?.setCamera({
  center: {
    latitude: currLocation.latitude,
    longitude: currLocation.longitude,
  },
  zoom: 14,
});

Hello, guys!
any ideas? I have the same issue


Review and Blog pages are not opening

Issue Description:
Review and blog pages are not working. Can u pls assign it to me?

@vishanurag I would like to work on this.Please assign me under the hacktoberfest tag