RFC for inclusive ranges with ...

Rendered

Imo we should try providing a (x..y).inclusive() method first. I'm also worried about potentially stealing the syntactic space for variadic generics

@Kimundi VG can still use <Args...> and (args...) because start... is not exactly meaningful (it goes up to infinity... inclusively?), though I expect ...end to work.

I want inclusive ranges, and I want the syntax to be x...y, because it's analogous with patterns. I also want exclusive patterns, but I'm content to wait. Similarly, I think we should support ...y. It seems clear that it is useful in the same way that ..y is useful. If I want the (bigint) numbers from 0 to n inclusive, I should be able to write ...BigInt(N), and not BigInt(0)...BigInt(N).
As for variadics, if the syntaxes turn out to be in conflict in some way, I'd rather have ... for inclusive ranges, given the precedent from patterns. I think it'd be more confusing otherwise. (My impression when we settled on changing patterns to ... was that we aimed to eventually support both inclusive/exclusive patterns/ranges).

Oh, also: I think that x...y should be an iterator, even if it requires an extra field. I think we should make it work as analogously to x..y as possible. Any discrepancies will be annoying warts. I don't think people store "inclusive ranges" as a datastructure very often (if ever), so the fact that the struct has an extra field shouldn't be a big deal. The usual way to use this is either as an iterator (in which case you want the field), or &vec[x...y], in which case hopefully everything is inlined and you don't ever create the struct in the first place.

Agreed with niko, though I'd be interested in investigating the API and performance impact of doing the hack I suggest to avoid a second field. You need to do a check in every step anyway. Might as well fold it into the add, right?
Things that come to mind:

You can't have inclusive range iterators of unit types (what a tragedy!)
I find the step API to be frustratingly opaque, but it seems like it has all the parts for doing checked adds. For types like BigInts these can return unconditional Some and therefore the checks should be const-folded fairly easy.

How common are inclusive ranges? I'm wary of putting a language sugar into places where a programmer benefits from it like once a year. (Library sugar like (x..y).inclusive() is ok.)
On the other side, inclusive ranges do improve consistency and completeness of the language, that already has inclusive range patterns and exclusive range expressions. On this ground, I'd suggest to extend the RFC with half-open inclusive range ...a and exclusive range patterns and position it as a complete "language beauty package".

@petrochenkov I agree that this works best if we have half-open inclusive ranges and exclusive range patterns as well. If we didn't already have inclusive patterns, I would prefer a method too.

CC #947

Yeah LGTM.

I've updated the RFC to include ...b, and to give a design that allows a...b to implement Iterator directly, to match a..b.

Hear ye, hear ye. This RFC is entering final comment period.

@huonw looks good to me.

I realize this problem already exists with patterns, but I don't like that ... is so close to .., visually. Mixing them up can result in off-by-one errors, one of two hard problems in computer science, so debugging them should not be made even harder by by making the two kinds of ranges identical from afar or in a hurry. To quote python-ideas (referring to Tim Peters):

Syntax must not look like grit on Tim's screen!

Therefore I favor (a..b).inclusive(), not just as conservative first step, but as permanent solution.

I think it's too similar to ..., I wouldn't add it just for the sake of consistency with patterns. Python, for example, only has exclusive ranges as well, and is one of the more convenient programming languages – if we introduce this I'd be in favor of a method like (a..b).inclusive(), because that makes it loud and obvious that this is not a normal range.

The libs team discussed this at triage today, and our conclusion was that we're ok with the structures defined here. The best route forward is probably to leave the finished field as #[unstable] for the time being to leave the door open to possible future improvements, but otherwise we're ok with it being public.

Will slices and vectors be indexed with inclusive ranges?

@bluss not sure; definitely not initially, and seems like that would be best be considered/proposed as a separate RFC.

Slice use of inclusive ranges seems to be an aspect that needs to be covered as well. Without it, this RFC adds an inconsistency. If we don't want to allow &foo[a...b], then maybe we should not have this syntax? Or will users be ok with the logic that half open ranges are index ranges, and closed ranges are not?

Heh, it didn't occur to me that this RFC wouldn't include slicing of slices/vectors with inclusive ranges. I do think we should support it, I think it'll be weird to have inclusive ranges but not support them in the area that is probably most commonly used, even if it eventually comes later. It feels to me like it should be a complete package.

I realize this problem already exists with patterns, but I don't like that ... is so close to ..

agree with that, how about ..~

Will slices and vectors be indexed with inclusive ranges?

I think they should not. I prefer the mental model where indices in a slice refer not to items but to positions between items. This makes the meaning of Index<Range> obvious. (Index<usize> gives the item after a position.)
https://blog.nelhage.com/2015/08/indices-point-between-elements/

I didn't think we meant to replace the current indexing with inclusive ranges, I thought maybe we could support both, is that not possible?

I didn't think we were saying we should replace the current indexing with inclusive ranges

Of course, removing existing Index<Range> impls would be backward-incompatible.

I thought maybe we could support both, is that not possible?

It is possible, multi-dispatch allows adding more impls. But that doesn’t necessarily make it desirable. Inclusive ranges have their use for iteration (e.g. for i in 0..255_u8) but how do they help for slicing, compared to "half-open" ranges? Again, think of indices as positions between items.
It’s also a matter of design principles, I come from Python with its There should be one-- and preferably only one --obvious way to do it zen.

Yeah I generally agree with the single way principle, I just personally don't really see it as much of a problem. Exclusive ranges would continue to be the norm the majority of the time, it just seems inconsistent that if you have b and you want to slice from a up to and including b, you have to do &vec[a .. (b + 1)] when you otherwise have this inclusive range syntax in the language that would allow you to do: &vec[a ... b]. I don't really see how it affects anything else, and would be nice and useful when needed, while omitting it would feel like an inconsistency/half-baked feature.

I really dislike the idea that ... exists but can only be used in a small and seemingly arbitrary subset of circumstances where .. can be used, despite both being so syntactically and semantically similar. It just seems super confusing.
If we use ..., I want it to do everything that .. can do. If we don't want to do that, let's please use a method. (a..b).inclusive() seems fine to me.

The motivation mentions loops from a to b inclusive, this can be covered with a simple adaptor method on the a..b range.
We already have [a, b) ranges, this rfc proposes to add [a, b] ranges (using [ for an inclusive end and ) for an exclusive/open end).  We already have use cases for (a, b) ranges too (BTreeMap's range and drain, and accessors), but that case is not resolved in this RFC. So the added [a, b] is neat, but does not give us the full power we actually need.

FWIW, if your only argument right now is for x in a...b, just do for x in (a..b).chain(Some(b)). Unless this comes up everywhere, I'm not really convinced we even need a method, never mind syntax.

@Veedrac I'd suspect that the chain solution wouldn't actually compile well enough. The inclusive range iteration needs its own iterator implementation to be efficient.

I looked for the obvious cases in rustc where this would be used.
No character ranges were found.
For a search of .. followed by + 1 (regex: [^.]\.\.[^.].*\+\s*1), and excluding all tests except for the shootout, we have three groups of usages. One is looping with for:

@bluss Suboptimal code is unfortunate, but this shouldn't always be the std's responsibility. It depends how common and how important to optimize the code is.

I looked for the obvious cases in rustc where this would be used.
No character ranges were found.
For a search of .. followed by + 1 (regex: [^.]\.\.[^.].*\+\s*1), and excluding all tests except for the shootout, we have three groups of usages. One is looping with for:
for deref in 1..autoderefs + 1 {
for i in 1..lines + 1 {
for _ in 0..(backslashes+1) {
for i in 1 .. iterations + 1 {
for i in 1..n as usize + 1 {
for j in 0..i + 1 {
for n in 1..max + 1 {
for i in 2 .. n_tasks + 1 {

The second is indexing:
for (place, val) in pp.iter_mut().zip(&self.perm.p[..i+1]) {
self.edges.$index(.. (pos + 1))
&ss[last_start..i + 1]),
let n = try!(self.inner.write(&buf[..i + 1]));
buf.push_all(&available[..i + 1]);
rotate(&mut perm[..i + 1]);
try!(wr.write(&line[..nb+1]));

And the third is iteration outside of for.
let messages = (min_depth..max_depth + 1).step_by(2).map(|depth| {
let mut dcol: Vec<_> = (0..t.len() + 1).collect();

This means 9/17 instances would soley need iteration, 7/17 would need indexing and 1 would need step_by. Not all of these would actually want to switch, though.

The tests gave these extra results:
for at in 0..len + 1 {
for i in 1..DATASZ + 1 {
for j in i+1..s.len() + 1 {
for len in 0..cap + 1 {
for tail_pos in 0..max_cap + 1 {
for to_remove in 0..len + 1 {
i += s[i + 10..].find("stack backtrace").unwrap() + 10;
let data = (0..4*100+1).map(|i| {
let data = (0..4*100+1).map(|i| {
let data = (1..DATASZ + 1).collect::<Vec<_>>();
let expected: Vec<_> = (0..i+1).collect();
let expected: Vec<_> = (0..i+1).collect();
let expected_left: Vec<_> = (-last..j+1).rev().collect();
let expected_left: Vec<_> = (-last..j+1).rev().collect();

for which only one required indexing.

Given this, I still think this is not common enough to have dedicated syntax.
If we do wish to have this in some form, it would seem an oversight to not include indexing, as about 40% of the use-cases are indexing. This means half-open ranges as well.
That said, the indexing case is already effectively bounds checked by construction so a +1 isn't too worrisome. That would leave no more than 10 discovered use-cases in the standard library, and some would be better off not switching.
I guess the point of what I'm saying is that I don't see the justification here. Having a subtle difference between ... and .. seems like an order of magnitude larger a problem than having slightly suboptimal inclusive ranges.

I do like the idea of having a lot of syntactic sugar, therefore I rate for .... I see the point, that inclusive ranges are not that common, but as rust depends a lot on ranges, still wanting to have maximum speed, I'd like to see ... becoming part of the language.
Featurewise ... should be equivalent to .. as otherwise there would be confusing discrepances.
My only problem is, that ... is not enough distinguishable from ... Therefore I'd also go with bombless's idea of ..~.
In conclusion I like the idea of inclusive range syntactic sugar, but the visual operator should be discussed more when the idea is through.

I'm also concerned that the syntax ... will be confusing, especially for either a) people new to the language or b) someone not familiar with rust doing maintenance on an existing code base. Off by one errors are hard to track down to begin with, let alone when you don't even know about the distinction between .. and ....
FWIW, i've been vaguely dabbling with rust for a few months now and its only after I saw this RFC that I realised why I could never remember if it was .. or ....
(I'm almost half tempted by the idea of [a..b), [a..b], etc.)

:+1: Since we already have ... for matches, I think it makes sense to have it for ranges, too.

My only problem is, that ... is not enough distinguishable from ..

... has one more dot than .., that is a significant difference.
Though : and ; do have insignificant difference, we still use them at the same time.
One more dot, one more element in range, which i like. +1 to ...

@liigo One dot is not a significant difference. Although one dot versus none may be, a sequence of identical symbols is hard on the eyes (as an extreme example, consider 99999999 vs 999999999). The comparison with : and ; does not hold water because the two are never interchangeable: First, context always tells me which one I should expect (foo(bar):, let x; i32;, let x: <statement>, field; Type, <T; Debug> and so on are all obviously wrong). Second, when I typo : for ; or vice versa and not noticed it (because, as you point out, they look quite alike), then the parser points it out for me. On the other hand, a..b versus a...b are very similar in all aspects: In many contexts not even the type checker can help me when I wrote one and meant the other, the mistake will only manifest itself as an off-by-one error.

If we’re going with anything else than ellipsis, I hope its .inclusive() method on the half-open range.

I agree with @rkruppe. This seems like a change that's guaranteed to cause a disproportionate number of off-by-one errors that will be difficult to identify and debug. If one were guaranteed never to accidentally type the wrong number of periods, this addition would be quite elegant and I definitely see the appeal of it. But as we are imperfect humans, ../... is a significance of syntax that I think Rust should avoid.
I also agree that an .incluse() method seems like the best alternative. Unusual syntaxes like ..= and ..~ are not aesthetically pleasing to me.

As a compromise option likely to please no one, we could just add even more periods to make them harder to confuse, like .... ;)

How will ambiguous syntax be parsed in case we pick ...?
Currently something like 2..... fails with error about finding a ..., however if ... for inclusive ranges is agreed upon, you could parse the previous expression as either (2..)... or (2...)... Will it make 2.... work (parsed as (2..)..)?

@nagisa This won't change the tokeniser (which is essentially what controls all those cases), so 2 ... .. for the first case, and error for the second (its token stream is 2 ... .)

What about [0..n]?

[0..n] is currently an array of type [Range<_>; 1]. While that's probably not very useful (so you could argue for breaking backwards compatibility here), it seems very strange to me to make a particular instance of array literals do something completely differently. And (0..n) would become misleading, it could then be interpreted as an open range. I also have doubts whether such syntax could be reconciled with the desire to need very little and constant lookahead in the parser.

I also think that .. and ... look to similar and would prefer the (a..b).inclusive() suggestion. So there is only one simple way to do these kind of ranges and it is clearer which range is actually used.
Additionnaly I think, that it also makes it easier and less confusing for new people to understand the two versions.

Typo waiting to happen. I suggest a..+b

@rhoark The problem with a..+b is, that it could be read by developers as a..abs(b).
Even though rust does not support this syntax, it could lead to misunderstandings for developers not familiar with rust.
Otherwise I would +1 a..+b :)

When I was writing Ruby code (a long time ago), I clearly remember being annoyed by the existence of two ellipsis versions (.. and ...) in the language. My problem was not to recognize which one was used (it's pretty clear when you use a monospace font), but to remember which one was the exclusive variant. Each time I read a line of code using it, I had to stop for 2 seconds to apply a mnemonic I made up, to answer the question "is this range inclusive or exclusive?".
The fact is, the meaning given to each variant is arbitrary. You could switch the two symbols and it would still be reasonable (like in Ruby). In current Rust, when I see an ellipsis I instantly know this is  an exclusive range, because the language has only exclusive range literals. To me, this is an easy rule to remember, because I know this that exclusive ranges are the most useful range type, and thus the logical choice. I admit this logic may not be as straightforward to a less experienced developer.
I think I like the proposed .inclusive() method. I don't have an opinion about the use of the ellipsis in patterns (never used them there).

+1 for .inclusive()

On 01 Sep 2015, at 16:46 , Olivier Renaud<EMAIL_ADDRESS>wrote:
When I was writing Ruby code (a long time ago), I clearly remember being annoyed by the existence of two ellipsis versions (.. and ...) in the language. My problem was not to recognize which one was used (it's pretty clear when you use a monospace font), but to remember which one was the exclusive variant. Each time I read a line of code using it, I had to stop for 2 seconds to apply a mnemonic I made up, to answer the question "is this range inclusive or exclusive?".
The fact is, the meaning given to each variant is arbitrary. You could switch the two symbols and it would still be reasonable (like in Ruby). In current Rust, when I see an ellipsis I instantly know this is an exclusive range, because the language has only exclusive range literals. To me, this is an easy rule to remember, because I know this that exclusive ranges are the most useful range type, and thus the logical choice. I admit this logic may not be as straightforward to a less experienced developer.
I think I like the proposed .inclusive() method. I don't have an opinion about the use of the ellipsis in patterns (never used them there).
—
Reply to this email directly or view it on GitHub https://github.com/rust-lang/rfcs/pull/1192#issuecomment-136727931.

(a..b).inclusive() is like a caricature of Rust verbosity.
I imagine that almost everyone would simply use a..b+1 for integers and (a..b).inclusive() for the rare occasion where that doesn't work, if they find the method in the docs before they hack something together.
For that reason I believe that an .inclusive() method would violate the single obvious way rule.

@Emerentius Wouldn't that still be a problem when having ...?

@Ticki Partially, yes. a..b+1 will always be the most obvious way, but the verbosity of .inclusive() will hinder adoption. You use it less, other use it less, you don't see it as often.

I do see use cases for inclusive ranges, but I don't see a clear winner in the proposed syntaxes. I especially dislike the .. vs ... syntax, for the following reasons:

As rkruppe points out, they are too similar. I'm afraid we're introducing off-by-one errors that are hard to spot.
The only other language I know that uses the ellipses is Ruby, and there .. is inclusive and ... exclusive, so the other way around. This gets confusing for some new users. I see at least one Rubyist that doesn't like their syntax for being too similar either.

I think .inclusive() is the best proposal here currently.
And perhaps this inspires some syntax ideas:
[a..b)    // from a (inclusive) to b (exclusive)
[a..b]    // from a (inclusive) to b (inclusive)

I don't think the above syntax clashes with existing bracket syntax (such as tuples or array indices) due to the presence of .. in there, but I'm probably wrong.

Actually I like [a..b] for exclusive and [a..b) for inclusive more, as [a..b] just looks better and is the 'normal' case, and ) just looks rounder, has a bigger stomach and therefore contains b ;)
One problem could be, that it breaks current programs, as a..b would not work anymore.
But in general +1 for this other syntax.

@Virtlink That clashes with how we parse the source code. The first thing that is done is parsing the brackets, and for this step they must match.

@Blackhole16 [a..b] and [a..b) are standard maths notation for inclusive and exclusive. See: https://en.wikipedia.org/wiki/Interval_(mathematics)#Including_or_excluding_endpoints

@erikjohnston oh, damn. I didn't say anything ;)
Then it's my most preferred syntax atm. Will [a..b) just result in an array then? As a..b does atm? Because in that case, that syntax makes even more sense then the current one, indicating that it is an array!

[a..b) is meaningless, because that’s exactly what a..b is and we ain’t getting rid of it;
[a..b] is a value of type [Range<T>; 1] and cannot be used for closed intervals;
(a..b) is a value of type Range<T> and cannot be used for open intervals;
(a..b] is the only thing that you cannot exactly write using any of current syntaxes without doing the arithmetic yourself.

[a..b] and (a..b) and [a..b) therefore are not an option.

We already have ... meaning inclusive for match patterns, so I think reusing that for inclusive ranges makes the most sense. I'd also like to see .. usable in match expressions.
For those who find .. vs. ... confusing in Ruby, I think this is at least partially because their meaning in Ruby is backward from what is (or at least what I find) to be intuitive.

@nagisa a..b could be syntactic sugar for [a..b), to be consistent. And in the other cases there is alternative syntax that's semantically the same as the existing syntax that would get replaced (e.g. [a..b; 1], when [a..b] denotes a closed interval).
However, this was just an idea. It may not be feasible from a backward-compatibility or syntactic standpoint. Perhaps there are other brackets that are similar enough and available, or there's a prefix, infix or suffix operator that can be used to distinguish between interval brackets and normal parentheses/brackets.

@rkjnsn +1 for a...b
Consistency with match patterns for both .. and ... will result in the least surprising behavior

@rkruppe

as an extreme example, consider 99999999 vs 999999999

You are just quibbling. Yes 99999999 vs 999999999 is not a significant difference, so does ........ vs .......... But we are comparing .. and ..., two dots and three dots, not many dots.

+1 for ... as inclusive.

While I agree that the current proposal may lead to confusion because of the small visual difference, I really don't think there's any way around this unless patterns are changed as well.
Otherwise, the resulting syntax asymmetry is simply much worse.

I think the fact that we already have ... patterns is what sways me towards that syntax as well.

My feeling is that we already had this debate back when we adopted .. (for ranges) and ... for patterns. I think the consistency there is important and worthwhile.

Huzzah! The language design subteam has decided to accept this RFC. There were two major points of contention:

whether or not ... ought to be an iterator, or merely support into iterator. The decision was to make ... an interator, which implies an extra field in the implementation to indicate termination.
whether to accept the ... syntax at all, or some other alternative such as an inclusive method. The conclusion was that consistency with ... patterns was desirable, as well as concise notation for use cases like vec.remove(1...5).

Awesome.

So, after the fact (sorry if this is way to late but this hasn't been stabalized yet)... How about using an enum to represent RangeInclusive? For example:
enum RangeInclusive<T> {
    Empty,
    NonEmpty {
        start: T,
        end: T,
    }
}

Rational:

finished is very iterator specific. Regardless of what happens, I think this field should be called empty.
start/end don't make sense if the range is empty. Using an enum prevents coders from using the start/end of spent ranges. Basically, this makes it impossible for the coder to do something like foo(my_range.take(10)); bar(my_range) and forget to check finished in bar.
If we ever get more space optimizations (specifically, utf8 code point ones) 'a'...'z' should be the same size as 'a'..'z'.


[Bug] alloy_sol_types::decode_revert_reason panics when called

Component
sol-types
What version of Alloy are you on?
├── alloy-sol-types v0.4.2 │   ├── alloy-primitives v0.4.2 │   │   ├── alloy-rlp v0.3.3 │   │   │   ├── alloy-rlp-derive v0.3.3 (proc-macro) │   │   │   ├── alloy-rlp v0.3.3 () │   ├── alloy-sol-macro v0.4.2 (proc-macro) │   ├── alloy-primitives v0.4.2 () │   │       ├── alloy-primitives v0.4.2 () │   │       ├── alloy-rlp v0.3.3 ()
Operating System
Linux
Describe the bug
Given this simple solidity contract:
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity ^0.8.17;
contract TestRevertMessage {

    error SenderAddressError(address sender);

    function revertWithSenderAddressError() public view {
        revert SenderAddressError(msg.sender);
    }
}

When the revertWithSenderAddressError() function is called, the transaction is correctly reverted. Nevertheless, trying to decode the revert message through decode_revert_reason causes a panic:
thread 'mycrate::test_decode_revert_reason' panicked at /home/myhome/.cargo/registry/src/index.crates.io-6f17d22bba15001f/alloy-sol-types-0.4.2/src/types/interface.rs:89:25:
attempt to add with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/std/src/panicking.rs:595:5
   1: core::panicking::panic_fmt
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:67:14
   2: core::panicking::panic
             at /rustc/cc66ad468955717ab92600c770da8c1601a4ff33/library/core/src/panicking.rs:117:5
   3: alloy_sol_types::types::error::decode_revert_reason
   4: mycrate::test_decode_revert_reason

Here is a small reproducer:
#[test]
fn test_decode_revert_reason() {

    // This is the hex-encoded output returned by RETH when the revertWithSenderAddressError() function is called.
    // RETH returns an `ExecutionResult::Revert { output, gas_used }` but `decode_revert_reason` panics when trying to
    // decode the output
    let revert_output_bytes_hex = "8758782b000000000000000000000000a48388222c7ee7daefde5d0b9c99319995c4a990";
    let revert_output_bytes = hex::decode(revert_output_bytes_hex).unwrap();

    let revert_message = alloy_sol_types::decode_revert_reason(revert_output_bytes.as_ref());

    println!("revert_message: {}", revert_message.unwrap_or_default())
}

@DaniPopes thanks for fixing it.
I noticed that with your fix the decode_revert_reason returns None while hardhat decodes that revert message to reverted with custom error 'SenderAddressResult("0x6D91e68e0adA18325a12Bd647A13342c141D0648")', is there a way to achieve a similar result with alloy?

Hardhat is not comparable to Ethers. Hardhat will compile all your contracts and keep their ABI in memory to use to decode errors and such. See the test I added on how to decode custom errors https://github.com/alloy-rs/core/blob/343f799b91a030874dba752fd543249c03f0c08b/crates/sol-types/src/types/error.rs#L496-L516


Nested div blocks don't fold correctly.

Quarto supports nested div blocks, described in https://pandoc.org/MANUAL.html#divs-and-spans
For example the following renders as expected:
# Nested callout

:::{.callout-note}

:::{.callout-note}

:::

:::{.callout-note}

:::

:::

However, in vscode, folding the top level div block doesn't hide both inner div blocks.

I think if possible it should look like the following when the top level div is folded.

If you use four colons for the outer div then this will work as expected:
# Nested callout

:::{.callout-note}

:::{.callout-note}

:::

:::{.callout-note}

:::

:::

This limitation arises because the markdown parser we use in vscode (markdown-it) isn't fully Pandoc compatible so there will sometimes be small differences in behavior.


finish all lab codes and additional for evaluation till Lab 6 on Wed, 20 Sept

9:00 AM, Wednesday, 20 September @aghogwarts


Aalen-Johansen fit() input - different cif table when I pass a numpy array vs pd.Series

import lifelines

def get_estimated_cif(durations, events, event_of_interest=1):
  ajf = lifelines.AalenJohansenFitter(calculate_variance=True)
  ajf.fit(durations, events, event_of_interest=event_of_interest)
  return ajf.cumulative_density_

get_estimated_cif(df["durations"].values, df["event"].values)
get_estimated_cif(df["durations"].values, df["event"])  # cif table is different from the table above

There are tied event times in my dataset - so _jitter is called.

The Aalen-Johansen estimator can't handle ties, as when events of different types occur, there needs to be a clear ordering for the computation. When AalenJohansenFitter sees ties, it randomly breaks them. The difference you see between a pd.Series and np.array is because the random number generator shifts the observations differently between the two calls.
If you want multiple calls to AalenJohansenFitter to produce the same result, you should manually break any tied event times in the data set. That way _jitter is not called, so the same event table should be produced each time.


Add a --help response to the CLI

Enable a user to run --help. Start off providing brief description of the project and how to run the script. Expand as needed.

Some users cannot use a GUI.
Add the optional functionality to run it via CLI, including a --help response


Issue with categorical variables in LGBM

Since i updated to the last version of the package (today), the TreeExplainer method does not work for categorical variables anymore. I am getting an error I was not getting before with the exact same code.
This is my code:
`data[cols_cat] = data[cols_cat].apply(lambda x: x.astype('category'))
X_train, X_test, y_train, y_test = train_test_split(data, y, test_size = 0.2, random_state=7)
dtrain_l = lgb.Dataset(X_train, label=y_train,
categorical_feature = cols_cat,
free_raw_data=False)
dtest_l = lgb.Dataset(X_test, label=y_test,
categorical_feature = cols_cat,
free_raw_data=False)
model= lgb.train(params, dtrain_l, valid_sets=[dtest_l], verbose_eval=10, early_stopping_rounds=10)
explainer = shap.TreeExplainer(model)
`
This is the error:
`---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
 in ()
2 # (same syntax works for LightGBM and scikit-learn models)
3 deb = time.time()
----> 4 explainer = shap.TreeExplainer(bestModel)
5 #shap_values = explainer.shap_values(X_train)
6 print(time.time() - deb)
L:\ProgramData\Anaconda3\lib\site-packages\shap\explainers\tree.py in init(self, model, **kwargs)
91             self.model = model
92             tree_info = self.model.dump_model()["tree_info"]
---> 93             self.trees = [Tree(e, scaling=len(tree_info)) for e in tree_info]
94         elif str(type(model)).endswith("lightgbm.sklearn.LGBMRegressor'>"):
95             self.model_type = "lightgbm"
L:\ProgramData\Anaconda3\lib\site-packages\shap\explainers\tree.py in (.0)
91             self.model = model
92             tree_info = self.model.dump_model()["tree_info"]
---> 93             self.trees = [Tree(e, scaling=len(tree_info)) for e in tree_info]
94         elif str(type(model)).endswith("lightgbm.sklearn.LGBMRegressor'>"):
95             self.model_type = "lightgbm"
L:\ProgramData\Anaconda3\lib\site-packages\shap\explainers\tree.py in init(self, tree, normalize, scaling)
380                             self.children_default[vertex['split_index']] = self.children_right[vertex['split_index']]
381                         self.features[vertex['split_index']] = vertex['split_feature']
--> 382                         self.thresholds[vertex['split_index']] = vertex['threshold']
383                         self.values[vertex['split_index']] = [vertex['internal_value']]
384                         self.node_sample_weight[vertex['split_index']] = vertex['internal_count']
ValueError: could not convert string to float: '3||4'
`

Sorry about that. #170 is the same issue. It has been fixed and will be released when v0.20.2 hits pip (later today).

Thank you very much !

The problem seems appear again. my version of shap is 0.24.0


Debugger is slow to respond to Pause request

Sending a Pause request (0x12) often isn't responded to until seconds later.  This is noted in the documentation, but it still seems excessive, as the usual use case for the request is to break into the debugger while a glitch is in progress.  Unfortunately, by the time Duktape responds, such a hiccup will likely have passed.
Would it be possible to reduce the delay to say, 100-250ms?

There are two issues affecting this: (1) bytecode executor interrupt mechanism and (2) artificial limits placed on frequency of debugger read polling.
The executor interrupt interval should probably not be an issue - though that depends on specific opcodes and native calls executed.
The debugger rate limiting is probably the cause, and it's "tuned" for a relatively lazy response to avoid excessive read polling (which is an issue for some transport where a poll operation actually needs to do something that matters). These artificial limits are here:
https://github.com/svaarala/duktape/blob/master/src/duk_heap.h#L263-L271
You can tune them directly from there; there's no external config option for them at the moment, though they'd be easy to convert into DUK_USE_xxx flags. If that was done, you could then tweak them manually from duk_config.h or bake into a duk_config.h using genconfig directly:
# Assuming DUK_USE_DBG_xxx config options were added
$ python config/genconfig.py --metadata config --output duk_config.h \
      -DDUK_USE_DBG_RATELIMIT_OPCODES=500 \
      -DDUK_USE_DBG_RATELIMIT_MILLISECS=50 \
      autodetect-header

Considering all socket reads on the target in my case are done asynchronously (non-blocking) and buffered, excessive polling shouldn't be an issue for me--it's just reading a variable, which is instantaneous.

That's often the case - but not always, which is why the default is ~200ms.

@svaarala So I've noticed with the latest master builds that the debugger appears to respond more quickly to Pause commands?  Has anything changed since 1.2 that might have reduced the latency?

Closing, the issue is understood and not a bug.

The possible concrete change would be to make those polling parameters part of duk_config.h so it would be possible to tweak them.

I'll reopen this because that config change might make sense.


Cake.CoreCLR can't handle whitespace in path

I know about #2214, and that seems to have been solved, but there is more to it:
Steps to reproduce:
mkdir 'My Folder'
pushd 'My Folder'
nuget install Cake.CoreCLR -o tools
dotnet new console -n HelloWorld
dotnet build .\HelloWorld\
echo 'DotNetCoreExecute("./HelloWorld/bin/Debug/netcoreapp2.2/Helloworld.dll");' > build.cake
dotnet .\tools\Cake.CoreCLR.0.32.1\Cake.dll
popd

What You Are Seeing?
No executable found matching command "dotnet-C:/Temp/My"
Error: One or more errors occurred. (.NET Core CLI: Process returned an error (exit code 1).)
.NET Core CLI: Process returned an error (exit code 1).
What is Expected?
Hello World!
What version of Cake are you using?
0.32.1
Are you running on a 32 or 64 bit system?
64
What environment are you running on?  Windows? Linux? Mac?
Windows
If 'My Folder' is changed to 'MyFolder', everything works as expected.

The problem seems to be in DotNetCoreExecutor.GetArguments(...)
where:
...
assemblyPath = assemblyPath.IsRelative ? assemblyPath.MakeAbsolute(_environment) : assemblyPath;
builder.Append(assemblyPath.FullPath);
...
should be:
...
assemblyPath = assemblyPath.IsRelative ? assemblyPath.MakeAbsolute(_environment) : assemblyPath;
builder.Append(AddQuotesIfRequired(assemblyPath.FullPath));
...
And AddQuotesIfRequired could be something like:
    private string AddQuotesIfRequired(string path)
    {
        return !string.IsNullOrWhiteSpace(path) ?
            path.Contains(" ") && (!path.StartsWith("\"") && !path.EndsWith("\"")) ?
                "\"" + path + "\"" : path :
                string.Empty;
    }

This goes IMHO for every use of assemblyPath.MakeAbsolute.

The PR failed during tests against other OS'es than Windows.
I think modifying the tests to expect quoted paths will also benefit the other OS'es, but I might not be the one to judge.

@Roadrunner67 I think qouting paths is generally a good idea, we do this in other places as well! I see tests are now passing on other OS’s in your PR.

fixed by #2511


Handling 3.X docker-compose versions

Hello,
I just encountered an issue with the plugin.
I have a docker-compose.yml opened in atom, then I try to select it with alt-shift-c, and I have an atom error :

I digged a bit, and found out something :

Apparently the services are retrieved for versions 1 and 3, but that does not work when your version is set as 3.7 (in my case):

A more complete services mapping would be appreciated :)
Thanks !

Same problem here!
Is this project dead?

I would be glad to give the hand to another core contributor, because I don't have time to maintain it, and I unfortunately don't use Atom regularly anymore.

It can be fixed by https://github.com/alanzanattadev/atom-docker/pull/26


Pretty printer test is very slow in windows only

The test PrettyPrinter.ReadAgainVeryShortLines took 8 seconds on Windows, but passed.  This test reads and pretty prints a single file, and 8 seconds seem pretty long.  In comparison this test takes 0.285 seconds on my desktop.
This could be a sign that something is wrong on windows or maybe unoptimized windows builds are just insanely slow.

The VeryShoftLines were made longer in
https://github.com/water111/jak-project/pull/69
and the test is a reasonable length now (< 1s).  The original lines were 5 characters which resulted in many line breaks, and is not handled efficiently by the pretty printer.
The windows/linux different is probably because msvc debug builds are very unoptimized and have extra checks.
The pretty printer being bad is tracked here: https://github.com/water111/jak-project/issues/63


Add python packaging support

Summary
This PR adds python packaging support, allowing the python OpenVDB bindings to be installed with pip. It also includes github actions to automatically publish releases to the Python Package Index (PyPI). See #161 for original discussion.
Changes

Added pyproject.toml, which specifies build-time dependencies for the python package.
Added setup.py, which makes pyopenvdb installable by pip, and specifies the python side of the build recipe.
Added a new github action to build a source distribution of the pyopenvdb package, and upload it to PyPI. The build can be triggered either by publishing a new OpenVDB release, or manually by navigating on GitHub to Actions->Build and Release Python Bindings to PyPI->Run Workflow and providing a branch and ref to build.

Additional Info
I explored a couple of options for generating binary python package distributions (wheels), but eventually opted only to generate source distributions (sdists). Here's what I tried:

cibuildwheel, a project by the Python Packaging Authority, allows a user to easily build wheels for python packages for a range of python versions, platforms, and system architectures. It does this by using docker containers to provide build environments. These manylinux docker containers can generate wheels for a wide group of linux distributions, but in opting for widespread compatibility they sacrifice being up to date:

We can't easily use manylinux2014 because it's based on CentOS 7, which only has boost 1.53 available in the official repositories, so we'd have to build it from scratch
We can't easily use manylinux_2_24 because it's based on Debian 9, and uses GCC 6.3 which can't build our dependencies.

I eventually gave up on cibuildwheel to explore other options.
2. Using ubuntu:22.04 to build wheels for Python 3.8, 3.9, and 3.10 seems like a good idea, and in fact I think it should in principle work - except we still need to build OpenVDB from source, which takes long enough on the github actions runners that the job times out.
So for this reason that I think it is best just to release an sdist to PyPI, which means that when a user installs the package with pip install pyopenvdb, pip will trigger a build of the shared library and copy it to the appropriate site-packages. sdists are commonly uploaded as "fallback" distributions on PyPI anyway, because they should be compatible with any system that has the appropriate dependencies, and are compiled at install time rather than at the time the package is published.
Update 2023-02-03
I've reworked this PR so that it now works with the new pybind11 bindings.
Changes

Added a workflow to build and publish an sdist package to either test.pypi.org (if a workflow_dispatch event is used ad the appropriate checkbox is clicked) or by default to pypi.org, the main python package index. Publishing requires the presence of a github secret with a PYPI token, named PYPI_API_TOKEN. If publishing to test.pypi.org, the secret that is used should be called TEST_PYPI_API_TOKEN I can meet to help out with this if the process is unclear. When a new release is made, this workflow will automatically build and publish the release to PYPI.
At the moment, there's no way to dynamically get the version with the build backend we are using; see https://github.com/scikit-build/scikit-build-core/issues/116 for more info. So whoever does the release will have to update the version number in pyproject.toml before publishing :/ hopefully this feature gets added to the build backend soon.
The python bindings will be published under the openvdb package name, as discussed before. To install, you'd just do pip install openvdb and to use them you'd do import openvdb.

Todo

[x] CLA
[x] Sign-off the commits
[x] Document requirements around using the github action to publish to PyPI (need to set up a PyPI token as a secret on the repository. OpenVDB maintainers should have access to the secrets on my fork, so you can see how to set it up.)

Hey @peytondmurray thanks a lot for your work on this, this is looking great! I need to spend a bit more time looking at what you have here but here's some initial comments:

Interesting that the cibuildwheel approach is locked to those older versions, though I see the reasons behind it. Out of interest what dependencies failed to build with GCC 6.3 with the manylinux_2_24 container?
Regrading the CI timeouts for ubuntu:22.04 that you were experiencing; we should be able to solve this. You can try turning -D USE_EXPLICIT_INSTANTIATION=OFF as this isn't really needed for the python bindings. Additionally, our actions support the use of a shared ccache to speed up subsequent builds. IIRC each job has a 6hr timeout limit? If we had a build matrix of a wheel per job (if such a workflow make sense with the wheel approach), combined with the above, I'd very much hope we wouldn't hit timeout limits!

I do like the idea of the sdist approach, though the idea of deploying binaries was pretty appealing to reduce the barrier to entry w.r.t build issues. I'm not sure if this is still enough of a reason to not go the sdist approach. Are their genral guidelines to choose one or the other? Do most pypi packages support both types of installations?
Finally we can absolutely credit Alex for their work! In case I haven't mentioned, the TSC have weekly catch-ups every Tuesday (10:00am to 11:00am (UTC-07:00)). They're minuted, but informal and open to anyone should you or Alex wish to attend and discuss anything here. Reminders are posted here: https://lists.aswf.io/g/openvdb-dev/topics. Either way I will feedback your progress, thanks again!

@Idclip Thanks for the info - I think based on your feedback I'll try once more to build wheels using ubuntu:22.04, knowing that it may lower the barrier to entry for python users.

Out of interest what dependencies failed to build with GCC 6.3 with the manylinux_2_24 container?

OpenVDB itself depends on GCC>=6.3.1.

Are their general guidelines to choose one or the other? Do most pypi packages support both types of installations?

Most large PyPI projects publish both wheels and sdists; the wheels are faster and smaller to install, but the sdists have better compatibility because the compilation step happens at install time. pip has flags for letting users choose whether to install sdists (--no-binary) or bdists (--only-binary) if they have a preference.

In case I haven't mentioned, the TSC have weekly catch-ups every Tuesday (10:00am to 11:00am (UTC-07:00)). They're minuted, but informal and open to anyone should you or Alex wish to attend and discuss anything here.

Hey, thanks for the invite! I saw that these meetings were on the calendar, but wasn't sure whether anyone would have questions about all this if I showed up. I'm traveling next week, but I think I might be able to make the meeting anyway. Even if nobody has any questions about this packaging stuff I'll come and say hello.

Okay, so I spent some time fiddling about getting wheels to build on ubuntu:22.04, and I'm getting close but I'm still not exactly sure how long it could take. There are still some difficulties getting cmake to find dependencies :upside_down_face: so if sdists are acceptable for now, I would like to break off support for building wheels into a separate branch to be added in a later PR. For now, I'll mark this as ready for review.

Okay, after looking more closely at this I've found the issue:
# openvdb/openvdb/openvdb/python/CMakeLists.txt
...
# @note  explicitly only search for Development.Module from 3.18 as searching
#   Development.Embed can cause issues on linux systems where it doesn't exist
if(${CMAKE_VERSION} VERSION_LESS 3.18)
  set(OPENVDB_PYTHON_REQUIRED_COMPONENTS Development)
else()
  set(OPENVDB_PYTHON_REQUIRED_COMPONENTS Development.Module)
endif()

I'm building with cmake 3.23.2 on my local machine, but it fails unless I force it to search for Development rather than Development.Module. @Idclip How would you like to proceed here? Checking the git log it looks like you might have run across some of these issues in the past, do you have any advice?

@peytondmurray I'm pretty confident that searching for Development.Module only is the right thing to do. I'm building with CMake 3.22.2 and have no issues. The fact that you're getting Could NOT find Python (missing: Development.Module Interpreter NumPy) to me implies something is wrong with your Python installation?

@Andrej730 I'm really sorry to have left this in a partially completed state for so long, so let me give a status update with some information:

I reached out to the owner of the openvdb package on PyPI, but have not heard back. If we are going to publish a package on PyPI for this, it feels like it could be useful to choose another name to publish it under; perhaps just openvdb?
It feels like a good time to start this work back up. I know there have been some discussions around using pybind11 and it would be useful to have CI set up for if/when that work moves forward.

Let me see if I can rebase onto the latest version and rebuild the python package. I'll spend a bit of time updating this PR, as I think a few of the github actions are now a bit out of date.

I reached out to the owner of the pyopenvdb package on PyPI, but have not heard back. If we are going to publish a package on PyPI for this, it feels like it could be useful to choose another name to publish it under; perhaps just openvdb?

If it would be possible to publish it as just openvdb it would be like the best outcome since that's how people would people usually go looking for the package after seing import openvdb somewhere in code.

Agreed, but it's really up to the maintainers. If I remember correctly the current python bindings are still being built to pyopenvdb, but it would be a really simple change to make this work. @Idclip thoughts?

Hi all, after a discussion with the maintainers at today's meeting, we have agreed to publish the python bindings under the openvdb name. However, this effort is currently blocked pending the merge of #1515. Once that PR is merged, this PR will be reworked to add the automation needed to build and publish the python bindings, which will no longer be dependent on Boost.Python :relieved: Thank you for your patience while this gets sorted out!

PR #1515 moved to #1571

Thanks @kmuseth and everyone else for the pybind11 effort, I'll rework this PR to get the build automation in place!

Well, the upshot of this taking so long is that the pybind11/cmake/scikit-build ecosystem has become significantly simpler to make use of since I was last working on this. There's now a scikit-build-core build backend that is used in a great new pybind11 example that made this straightforward. Thanks to all who are working on making those projects so well documented and easy to use!
I tested and was able to publish an sdist on test.pypi.org, and was able to install it with pip install. However, I kept getting a segfault whenever I tried to import the bindings - I assume this is some pybind11 issue, but anyway I didn't investigate too much yet.

Currently blocked by https://github.com/AcademySoftwareFoundation/openvdb/issues/1583.

Oh, and I forgot to mention that it would be great if the package's metadata included some urls.

@JeanChristopheMorinPerso Thanks for taking a look at this. URLs have been added.
About building precompiled binaries: you can read through the comment history above, but the reason I didn't do this before was effectively because of the dependency on Boost.Python; openvdb required a relatively newer version which wasn't available on the CentOS 7 (?) docker images that everyone uses for manylinux builds. This resulted in dependency hell trying to get the right version of boost with the right version of gcc, and everything needed to be compiled which ran up against CI runner timeout limits, and so on and so on.
The upshot of dropping Boost.Python as a dependency and swapping to pybind11 is that we don't have to worry about this. I'll get to work building this with cibuildwheel, which should make the job pretty simple.
I'm also seeing the issue with the include you saw above. This wasn't happening last time I worked on this, so I assume it's part of the growing pains associated with adding pybind11 support. I don't think it's related to the build system changes introduced here.
Thanks again for the input :rocket:

Great @peytondmurray! I wasn't sure if boost was used for something else than the Python bindings (previously). It's good to know it was only used for that and that it's no more required.
Feel free to poke me when you'll have wheels built by GitHub Actions. I'll happily review the changes and the produced wheels on all platforms (I usually download the wheels and inspect the binaries manually, and then install on all platforms in different scenarios to see if everything holds up together).

@JeanChristopheMorinPerso as expected the pybind11 dependency makes everything way easier to set up with cibuildwheel. Right now I'm blocked on the build failure you saw above, but I'll also see if I can't find a fix for that in a little bit - then we should be able to merge this.

Hi. Has there been any progress on this issue?
I've looked into it a bit, and the problem I see is that you're trying to build the openvdb libs + bindings directly using the openvdb/openvdb/python/CMakeLists.txt as the entry point in the GitHub actions script. However, when reaching this line of the build script, openvdb as a CMake target is not yet defined. That's why one should always link against namepaced targets in CMake. Indeed, in this case, CMake did not find any target named openvdb in scope, so it will simply add -lopenvdb to the link flags for the _openvdb target and fail at link time. Here we fail sooner because the transitive include directories for the openvdb target are not passed down to the client lib _openvdb. Again this is not a problem when building the bindings from the root CMakeLists.txt, because by the time we reach this line openvdb is a well-defined CMake target, and will bring its transitive list of include directories necessary for compilation.
So my recommendation would be twofolds:

Use an namespaced ALIAS target here, e.g. OpenVDB::OpenVDB, and replace all usages of openvdb with this new namespaced target.
Add the following code snippet in this line:if(NOT TARGET openvdb)
  add_subdirectory(../../.. openvdb_build)
endif()

This will allow using openvdb/openvdb/python/CMakeLists.txt as the entry point for the build script.

This still doesn't work for me, as it's looking for BLOSC and doesn't find it, but I'm guessing by disabling the corresponding options you could get it to work eventually.
 Also I think a maintainer needs to approve the workflow for the build process to be triggered...

@jdumas I meant to post this yesterday, but thanks for looking into this. I committed your suggested changes yesterday, then tried to build the python bindings. It was taking too long on my laptop, so I pushed the changes with the intent to build when I got home, but never got around to taking care of this. I'll test it now, then sign the commit in a few minutes.

I was able to build it on my machine with the following changes to your branch:
diff --git i/openvdb/openvdb/python/pyproject.toml w/openvdb/openvdb/python/pyproject.toml
index 9876bd04..bd8bc50b 100644
--- i/openvdb/openvdb/python/pyproject.toml
+++ w/openvdb/openvdb/python/pyproject.toml
@@ -40,3 +40,4 @@ cmake.minimum-version = "3.18"
 [tool.scikit-build.cmake.define]
 OPENVDB_BUILD_CORE = "ON"
 USE_NUMPY = "ON"
+USE_BLOSC = "OFF"
diff --git i/openvdb_cmd/CMakeLists.txt w/openvdb_cmd/CMakeLists.txt
index c4351f49..f5cd7cda 100644
--- i/openvdb_cmd/CMakeLists.txt
+++ w/openvdb_cmd/CMakeLists.txt
@@ -42,7 +42,7 @@ if(NOT OPENVDB_BUILD_CORE)
   # both cases
   set(OPENVDB_LIB OpenVDB::openvdb)
 else()
-  set(OPENVDB_LIB openvdb)
+  set(OPENVDB_LIB OpenVDB::OpenVDB)
 endif()

 set(OPENVDB_BINARIES_DEPENDENT_LIBS

Yep, I think I just missed a few places which needed the new alias (I didn't need to fiddle the BLOSC flag). I was able to successfully build it, so I've signed off now and pushed the new version.

@peytondmurray let me know when you are ready for another code review.

Thinking about this PR again I think we might want to use the alias name OpenVDB::openvdb to be consistent with the result of the FindOpenVDB.cmake provided in this repository. Although ideally we shouldn't write any FindFoo.cmake by hand, and we should instead rely on exported FooConfig.cmake generated by CMake, such a change would be beyond the scope of this PR.
It'd be great if we could:

Update the alias name from OpenVDB::OpenVDB to OpenVDB::openvdb to be consistent with the current FindOpenVDB.cmake.
Have a maintainer approve the workflow to make sure it works properly.

I'd like to help move the needle on this, so please let me know if there are other ways I can help.

@jdumas Thanks for following up about this. Following your suggestion, I've changed the alias to OpenVDB::openvdb. Now we need to start running the CI to make sure this works. @JeanChristopheMorinPerso could you please trigger a workflow run to see if cibuildwheel succeeds?

I don't have the permissions to trigger the workflows. I'm only a bystander that is helping doing code review on this PR.

Ahh :sweat_smile: sorry! @Idclip Would it be possible to retrigger CI?

@peytondmurray have you beem able to address @JeanChristopheMorinPerso's lastest comments? We still need a maintainer approval to run the github action workflows for this PR.

@AcademySoftwareFoundation/openvdb

@jdumas Yep, I think the comments above are addressed.

@Idclip Can you re-trigger CI please?

@Idclip Can you re-trigger CI please?

Worflow flie seems to be invalid: https://github.com/AcademySoftwareFoundation/openvdb/actions/runs/5639306194/workflow

It looks like the github action file is missing a jobs: section before the publish: job.

@peytondmurray sorry to ping you again, but would you be able to update your github action file to correct the syntax by adding the jobs: section?

Yep! Sorry, the last notification got buried, I'll do it now!

Argh it looks like it needs approval again from a maintainer to run :/. Hope it works this time.

Yeah, I wish I'd made a smaller/easier to merge PR before submitting this one so we didn't have to wait for the workflow checks - I think they're only guarded against first time contributors. Thanks for your patience!

I'm not sure that it needs to be approved again for every commit of the same PR though, that seems a bit too much.

I'm not sure that it needs to be approved again for every commit of the same PR though, that seems a bit too much.

Not sure we can change this (?) but I'll try to remember to press the button when I see notifications

Hey folks, it's been a while since I updated this PR. At this point I'm open to PRs against this branch, or if someone wants to take over pushing this forward. At the moment I've been preoccupied with other work, and don't have much time to press this forward much more. What's the best way to tie this up? I'd be happy to do something here if it's small, but otherwise would it be best to close it out? I'm not sure how much need there is for this right now.
I'd also like to point out that meson-python has been really useful in a couple of projects that I've worked on in building compiled python extensions; it's much easier to deal with than cmake IMO. It might be worth looking into if fiddling with cmake is a challenge here.

@peytondmurray could you give me write access to your fork? I don't have a lot of time right now, but I'd be happy to contribute when I can. One thing that makes it slow to iterate on this PR is that each commit requires approval from a maintainer before running GitHub actions. However, I think it only does that for first-time contributors, and I already have existing contributions to this repo, so it's possible that any of my commit triggers GitHub action right away (haven't been able to test that on @peytondmurray's branch since I don't have write access to it).

Invite sent!


Provided demo code never outputs anything to the console.log()

No matter what events are used on-transition-left vs on-swipe-left etc. none ever get called to output the demo debug text to the console.log
Looking at the code i'm assuming as cards are swiped off new cards should be added? This too doesn't happen so I eventually get to the end of the card pile.
I'm testing in the Chrome browser. But even on iOS with Ionic View I can get to the end of he card pile as no new cards are being added.
The only output I do see in the console is regarding:
'webkitMovementX' is deprecated. Please use 'movementX' instead. ionic.bundle.js:1162
'webkitMovementY' is deprecated. Please use 'movementY' instead. ionic.bundle.js:1162

This may be due to a new version of Ionic.

I''m not sure how is it supposed to work, but I guess it's because "cardSwipedLeft" and "cardSwipedRight" are called outside the "CardCtrl" Controller. When I move them into the "CardsCtrl" everything works fine.

@abimelex you are completely correct with your observation and all event output to console works by moving them to the parent controller.


Support configuring the user id

Hi, this is a feature request to support configuration of the fields hardcoded here:
https://github.com/Mic92/ssh-to-pgp/blob/1ce63fee58e54a5133eccb12993588d46b3dfeab/convert.go#L44
The main idea is that, when managing multiple keys from different hosts, it's difficult to tell which key belongs to which host at a glance. For my use case it would be sufficient if localhost would default to the hostname of the current machine, but perhaps allowing some/all fields to be specified via command line arguments would be more extensible.

Patches are welcome. I don't think I will implement this myself since I mainly use ssh-to-age for new machines.
The code should be simple enough. You can start by adding a hostname here: https://github.com/Mic92/ssh-to-pgp/blob/main/main.go#L17

This was implemented.


GitLab extension to fetch the repos proactively

Extension – GitLab
Author: @tonka3000
Currently, when I list my projects using the GitLab extension, the extension makes a request when I attempt to do that, which means I have to wait until the API responds, and it is usually slow. In my day-to-day usage, I really want to be able to use this extension to quickly jump to my repo, but sometimes it is even slower than actually going to GitLab UI and searching there.
Describe the feature and the current behavior/state.
I think the GitLab extension should fetch the repositories in the background, just like the GitHub extension does. I can search my GitHub repos very quickly, but not for GitLab.
Who will benefit with this feature?
All the users of GitLab extension, in my opinion.

Hi @karakanb ,
thanks for reaching out to me.

In my day-to-day usage, I really want to be able to use this extension to quickly jump to my repo, but sometimes it is even slower than actually going to GitLab UI and searching there.

Can you describe this in more detail. The GitLab extension already using a SWR cache, so only the first call should take longer.
How often do you open raycast to get to your GitLab repo? Only a few times?
The cache right now fetching all projects in the beginning, so it can be locally searched afterwards (this is what you see when you mention that the GitLab webui is faster, because it loads only the first page of results and the extension fetch all in the beginning). After the first call the projects will update will happen in the background (when you open the command, there are no background service in raycast right now) and searched should be fast, more or less like the GitHub extension.
Cache
The cache itself will refetch in the background when the local data is older than 5 minutes and the cache is invalid after 24 hours. Invalid mean that the extension refetch all new and not in the background. The 24 hours is also a moving time-window, so when you open the projects command often enough the cache should not be invalid. Maybe 24 hours is to short for your actual use case.
So I can make this settings user editable, so that you can increase the invalidation time, so that e.g. your get invalid after  7 days or so. What do you think, would that match your daily usage pattern?

Hey, thanks a lot for the details. My usage is usually 1-2 times a day, and somedays many times a day when I focus on some of those repos.
I have been guessing that there was a cache like that because I noticed that when I loaded it up afterward it loads quickly. I think the repo information is something that changes rarely in the majority of the cases, so I have two ideas:

Make the cache configurable, as you said.

At least making it a longer period could work by default.
Also, I think refreshing this every 5 mins is too often anyway, right? maybe if that was also configurable or if I had the ability to trigger a refresh manually, I could very well live with an hourly or a few hours of refresh interval.

Make the list show stale data even when it is refreshing the cache.

This would still allow a fluid user experience while serving up-to-date information.
I think Raycast does something like this while serving the store page, so I presume the ability to do that is there.

Thanks for looking into this.


 We should be able to hide them behind a let binding.

yeah - this is definitely still a work in progress.


WIP: Updated daemonset for taints and tolerations.

Fixed part of #2737

This change is

FYI: @mburke5678

I noticed another thing we need to change in that doc, so I just went ahead and made all of the changes in a new PR #2905 so I'm going to close this one.


PYTHON (KERAS) - How do I modify KERAS builtin “get batch” callback function?

Hi, everyone.
I am currently working on an image classification problem for my PhD Thesis on oil/gas data processing. The data I need to classify is a SINGLE image from a borehole (like this one here, on the left: http://hager-richter.net/wp-content/uploads/2012/03/bore_graf.jpg), from which I have a certain lithological classification curve.
This means that, on one side I have a continuous image (like the one on the link), and on the other side I have a discrete curve with categorical numbers (say, 0, 1, 2, 3..., where each number indicates a certain lithological class, something like the type of rock that line of the image can be classified as). Let's call the input data (image) "X" and the classification curve "Y".
X has a shape of: (img_Height, img_Width, number_img_channels)
Y has a shape of: (img_Height, 1)
The idea is to use a CNN (raghakot resnet implementation: https://github.com/raghakot/keras-resnet) to create a model that petrophysicists can use to classify other borehole images.
Now, if I'm not mistaken, any image data inputed into a keras model should have the shape (number_of_samples, img_Height, img_Width, number_img_channels), so in order for this to work I need to modify the keras built-in batch function so that instead of getting a batch by taking something like:
Xbatch = X[batch_index,:,:,:]
it actually uses a certain window around each "batch_index", so that the function returns something like:
H = 21 #window height
Xbatch = np.zeros((len(batch_index)-1, img_Height, img_Width, number_img_channels)
for i in np.arange(0,len(batch_index)-1):
   Xbatch[i,:,:,:] = X[batch_index[i]+H, :, :].reshape((1, H, img_Width, number_img_channels))

The problem is that I have not idea which is the function (or file) of the keras module that I should modify in order to make this work. I think it might have something to do with the "keras.preprocessing.image.ImageDataGenerator" function, but I have not been able to find the right place where I should put my code.
This is the part of my code that initializes the model and trains it, using data augmentation (copy/pasted from raghakot implementation):
# Model definition
        model = resnet.ResnetBuilder.build_resnet_18((img_channels, __M__, __N__), nb_classes)
        model.compile(loss='categorical_crossentropy',
                      optimizer='adam',
                      metrics=['accuracy'])

        if not data_augmentation:
            print('Not using data augmentation.')
            model.fit(XTrain, YTrain,
                      batch_size=batch_size,
                      nb_epoch=nb_epoch,
                      validation_data=(XTest, YTest),
                      shuffle=True,
                      callbacks=[lr_reducer, early_stopper, csv_logger])
        else:
            print('Using real-time data augmentation.')
            # This will do preprocessing and realtime data augmentation:
            datagen = ImageDataGenerator(
                featurewise_center=False,  # set input mean to 0 over the dataset
                samplewise_center=False,  # set each sample mean to 0
                featurewise_std_normalization=False,  # divide inputs by std of the dataset
                samplewise_std_normalization=False,  # divide each input by its std
                zca_whitening=False,  # apply ZCA whitening
                rotation_range=0,  # randomly rotate images in the range (degrees, 0 to 180)
                width_shift_range=0.1,  # randomly shift images horizontally (fraction of total width)
                height_shift_range=0.1,  # randomly shift images vertically (fraction of total height)
                horizontal_flip=True,  # randomly flip images
                vertical_flip=False)  # randomly flip images

            # Compute quantities required for featurewise normalization
            # (std, mean, and principal components if ZCA whitening is applied).
            datagen.fit(XTrain)

            train_generator = datagen.flow(XTrain,YTrain,batch_size=batch_size)
            # Fit the model on the batches generated by datagen.flow().
            model.fit_generator(datagen.flow(XTrain, YTrain, batch_size=batch_size),
                                steps_per_epoch=XTrain.shape[0] // batch_size,
                                validation_data=(XTest, YTest),
                                epochs=nb_epoch, verbose=1, max_q_size=100,
                                callbacks=[lr_reducer, early_stopper, csv_logger])

Thanks in advance!

Looks like _flow_index in ImageDataGenerator is the function that handles the slicing of the arrays to batches  - you should probably look to modify it but I might be wrong

First of all, thanks for your comment @tRosenflanz!
As suggested, I tried to find out whether _flow_index was the function that iterated the batches on my code, so I simply added a print line at the beginning of it. However nothing appeared on the console when I called the function again so I tried to take a look a little bit further into it and found out that the function that is handling the batch iteration is the NumpyArrayIterator. I placed a print('HEY!') right after its __init__ definition and it did print the message on the console.
Thus, I am going to modify the ImageDataGenerator function and add a new class (say, NumpySlidingIterator) and see if I solve this problem.
I'll post any news regarding this issue.

If anyone is interested in this, I was able to solve the problem by creating a copy of "keras.models", which I called "SlidingModels", and then adding five functions inside the Model class definition: _fit_sliding_loop (a modified copy of _fit_loop function), _predict_sliding_loop (a modified copy of _predict_loop), _test_sliding_loop (a modified version of _test_loop), fit_sliding (a modified version of fit function) and predict_sliding (a modified version of predict function).
A part from that I had to create the function _slice_sliding_arrays (modified copy of _slice_arrays function), outside the Model class definition.
Doing that I was able to train my CNN using a sliding window. The main stuff is on _fit_sliding_loop, inside the for batch_index, (batch_start, batch_end) in enumerate(batches):, where I added the sliding window cropping of the data. Now this loop looks like this:
`if shuffle == 'batch':
index_array = _batch_shuffle(index_array, batch_size)
elif shuffle:
np.random.shuffle(index_array)
            batches = _make_batches(num_train_samples, batch_size)
            for batch_index, (batch_start, batch_end) in enumerate(batches):
                batch_ids = index_array[batch_start:batch_end]
                try:
                    if isinstance(ins[-1], float):
                        # Do not slice the training phase flag.
                        # slice x
                        start = batch_ids.tolist()
                        start = [training_indexes[s] for s in start]
                        xbatch = np.expand_dims([x[s:s+sliding_window,:] for s in start],axis=3)
                        ybatch = y[start,:]
                        ins_batch = (xbatch,ybatch,(ins[-2][batch_ids]),(ins[-1]))
                    else:
                        start = batch_ids.tolist()
                        start = [training_indexes[s] for s in start]
                        xbatch = np.expand_dims([x[s:s+sliding_window,:] for s in start],axis=3)
                        ybatch = y[start,:]
                        ins_batch = (xbatch,ybatch,(ins[-2][batch_ids]),(ins[-1]))
                except TypeError:
                    raise TypeError('TypeError while preparing batch. '
                                    'If using HDF5 input data, '
                                    'pass shuffle="batch".')
                batch_logs = {}
                batch_logs['batch'] = batch_index
                batch_logs['size'] = len(batch_ids)
                callbacks.on_batch_begin(batch_index, batch_logs)
                for i in indices_for_conversion_to_dense:
                    ins_batch[i] = ins_batch[i].toarray()

                outs = f(ins_batch)
                if not isinstance(outs, list):
                    outs = [outs]
                for l, o in zip(out_labels, outs):
                    batch_logs[l] = o

                callbacks.on_batch_end(batch_index, batch_logs)
                if callback_model.stop_training:
                    break

                if batch_index == len(batches) - 1:  # Last batch.
                    if do_validation:
                        val_outs = self._test_sliding_loop(x=x,y=y, validating_indexes=validating_indexes,val_f=val_f, val_ins=val_ins,
                                                   batch_size=batch_size, sliding_window = sliding_window,
                                                   verbose=0)
                        if not isinstance(val_outs, list):
                            val_outs = [val_outs]
                        # Same labels assumed.
                        for l, o in zip(out_labels, val_outs):
                            epoch_logs['val_' + l] = o`

Anyway, If anyone wants more information about the code, send me a message.


validate_endstate_energies_point failing on openmm nightly

test_PointMutationExecutor_endstate_validation and a few other tests are failing now on the nightly tests for openmm:
             print(f"{components_other[i][0]} -- og: {components_other[i][1]}, hybrid: {components_hybrid[i][1]}")
>           assert np.isclose(components_other[i][1], components_hybrid[i][1])
E           AssertionError

perses/tests/utils.py:964: AssertionError

(a few tests are also failing with a force index error, but that is fixed by PR #976 )

I was digging into this and I think it is actually the same issue with the ordering of the forces and getting them by index instead of by name. One can see the differences here:
# OMM 7.7

components_other =
[('HarmonicBondForce', 0.5488766596317677),
 ('HarmonicAngleForce', 0.9512153356249284),
 ('PeriodicTorsionForce', 14.716772049541484),
 ('NonbondedForce', -50.710727641191205),
 ('AndersenThermostat', 0.0)]

# OMM 7.8 Nightly

components_other =
[('HarmonicBondForce', 0.5879495208741793),
 ('PeriodicTorsionForce', 14.770935074911064),
 ('NonbondedForce', -50.78580169291506),
 ('HarmonicAngleForce', 0.9146367896110912),
 ('AndersenThermostat', 0.0)]

It should be solved by accessing the components by name instead of index. I think the way to go is to use a dictionary in compute_potential_components util here. We then need to adapt the code wherever this function is used.
On a side note, I'm not even sure why compute_potential_components is on test/utils.py when it's being used as a functional part in the code base outside of the testing.

@ijpulidos Check where it is used and move it somewhere else that is more appropriate.


WIP: Add spinner during locking

Signed-off-by: Dan Ryan<EMAIL_ADDRESS>

Closing, I have a better implementation of this


Add two factor authentication plug-able modules

Description:
Add two factor authentication support for auth_provider.
Auth module
Two auth modules has been added to auth_providers/modules folder.
totp is Time-based One Time Password module which compatible with Google Authenticator and Authy (not test yet).
insecure_example is a Example designed for demo and unit testing purpose, should not be used in any production system.
Auth module can be used mixed-match with auth providers. After normal auth provider validate, if there are auth modules configured, the login flow will direct to auth module input form by configured order. Auth module use separated storage, can be hardening it in future.
Changing to add user script
Right now, we still don't have UI to create user, the only way is using script documented at #14365. In case to enable 2FA, I make following change:
Add a optional argument -t True if you want to enable 2FA, so for create new user, you need
> hass --script auth -c your_config_dir -t True add your-user your-pass
User created, please set up Google Authenticator or any other compatible apps like Authy with key: SOMEKEY

Script will generate a KEY for that user, you can use this KEY to create an entry in Google Authenticator

Then you can test your Time-based OTP by
hass --script auth -c your_config_dir -t True validate your-user your-pass --code YOURCODE

Related issue (if applicable): fixes #
Pull request in developers.home-assistant.io with documentation (if applicable): home-assistant/developers.home-assistant#52
Example entry for configuration.yaml (if applicable):
# configuration.yaml
homeassistant:
  # Auth Provider
  auth_providers:
    - type: homeassistant
      modules:
        - type: totp

auth:

Checklist:

[x] The code change is tested and works locally.
[x] Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

[x] Documentation added/updated in developers.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

[x] New dependencies have been added to the REQUIREMENTS variable (example).
[x] New dependencies are only imported inside functions that use them (example).
[x] New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

If the code does not interact with devices:

[x] Tests have been added to verify that the new code works.

For other auth provider, such as legacy_api_password, you need to manually edit ~/.homeassistant/.storage/auth_module.totp file (the username for legacy_api_password is homeassistant)
To manually generate ota_secret, you can execute following python script
import pyotp
print(pyotp.random_base32())

It's usually wise to discuss these things before creating big PRs like this. That way there is no work wasted if we have different views on the architecture.

I thought we had the discussion around plugable auth module in #15225

That's true, but that's only 1 part of this. Starting code review now

Hello @awarecan,
When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).
The commits that are missing a linked GitHub account are the following:

6f9a98b674fcefd9e2d5165915da4db18529ee30 - This commit has something that looks like an email address (ec2-user@ip-172-31-28-229.us-west-2.compute.internal). Maybe try linking that to GitHub?.

Unfortunately, we are unable to accept this pull request until this situation is corrected.
Here are your options:

If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.

If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.

If you only made a single commit you should be able to rungit commit --amend --author="Author Name<EMAIL_ADDRESS>
(substituting Author Name and<EMAIL_ADDRESS>for your actual information) to set the authorship information.
If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:

You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.

Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.
Thanks, I look forward to checking this PR again soon! :heart:

Half-way in the refactoring

[x] changed the directory structure
[x] remove session
[ ] move auth module config out of auth provider
[ ] change login flow to allow user select auth module

Code is ready for review, document still need update

This PR should be split in three parts.

Refactor auth
Add mfa and just add insecure example to show how it works
Add totp mfa module

Right now it's not clear how the design completely works.

I've merged a reorg of the auth in #15443 . I've used git mv so the changes should be minimal. I had little trouble rebasing #15420

(make sure you squash all your commits before the rebase)

I will merge my user management, that way it will slim down this PR even further, making it more visible what things are added to support MFA.

The current architecture is not correct. Makes incorrect assumptions about auth providers and how things relate to each other.
I have currently spend too much time on reviewing this PR and it has distracted me from my actual priorities. I won't be looking at this PR for the upcoming week.

The core issue here is:
Current flow: after auth provider validate, we got flow result, starts from here, flow is finished, control back to auth manager, then it will use flow result to get or create credential, then get or create user, then login finish.
The challenge is, in mfa module, it needs user_id, we have do some hack here to try to get user by mfa module. That is the core issue.
If we want to do it in right way, we need to change data flow manager, to handle more flexible way, for example, I will suggest change FlowManager to
class FlowManager:
    def __init__(self, hass, async_callback_flows: dict):

and the async_callback_flows should be something likes
async_callback_flows = {
    'init': async_init_flow
    'get_user': async_get_user_flow
    'finish': async_finish_flow
}

Rebase seems mess up, close this PR for now


quarkus-full-microprofile fails on quarkus main with GraalVM for JDK 21 on windows.

Error:  Failed to execute goal on project quarkus-full-microprofile: Could not resolve dependencies for project org.graalvm.tests.integration:quarkus-full-microprofile:jar:1.0.0-SNAPSHOT: Could not find artifact io.quarkus:quarkus-smallrye-opentracing:jar:999-SNAPSHOT -> [Help 1]
Error:
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/DependencyResolutionException
Error:  Tests run: 2, Failures: 0, Errors: 1, Skipped: 1, Time elapsed: 7.833 s <<< FAILURE! - in org.graalvm.tests.integration.RuntimesSmokeTest
Error:  quarkusFullMicroProfile{TestInfo}  Time elapsed: 7.833 s  <<< ERROR!
java.lang.IllegalArgumentException: Path to D:\a\mandrel\mandrel\apps\quarkus-full-microprofile\targetmust exist and regexp must nut be null.
	at org.graalvm.tests.integration.RuntimesSmokeTest.testRuntime(RuntimesSmokeTest.java:98)
	at org.graalvm.tests.integration.RuntimesSmokeTest.quarkusFullMicroProfile(RuntimesSmokeTest.java:192)

Looks related to a comment @zakkak made elsewhere:
smallrye-opentracing no longer being present https://github.com/quarkusio/quarkus/pull/36602 (native-tests.json needs adjustment, PR handling it https://github.com/quarkusio/quarkus/pull/37101)
Not sure how a fix would look like in integration tests. Added dependency for quarkus > someversion?

See: https://github.com/graalvm/mandrel/actions/runs/6882513424/job/18721824498#step:10:7503

@Karm Could you please look at this with some priority? This is generating a lot of noise.

Temporary work around to get the CI green again: https://github.com/Karm/mandrel-integration-tests/pull/224

Hmm, how about we produce a profile that activates based on the quarkus version and adds the quarkiverse dependency for later versions?

No, I haven't.
The main issue I see with that is that our test will then depend on a quarkiverse dependency which I expect to be tricky when testing Quarkus main.
Since I am not familiar with the code nor with the testing goals of it I will leave that to @Karm. Perhaps we can completely replace it or drop the smallrye-opentracing dependency.

Closing this one as #225 tracks the remaining issue as #224 disables the tests.


Bug: tf.cumsum is not numerically stable when sequence is long.

System information

Have I written custom code: yes, but very little
OS Platform and Distribution: CentOS 5
TensorFlow version (use command below): both 1.18 and 1.12
Python version: both python 2 and 3
GPU model and memory: TitanX and 1080Ti

Describe the current behavior
cumsum has small (possibly rounding or something similar) errors, and these errors accumulates quickly as the sequence length increase, e.g. a time sequence of length 1000.
Describe the expected behavior
cumsum should be as accurate as the float32 precision.
Code to reproduce the issue
Here is the colab gist to reproduce the result
https://colab.research.google.com/gist/rex-yue-wu/cb19dc2af6f8709002b6d412611e972a/unstable-tf-cumsum.ipynb

@azaks2 can you or someone else take a look?

A simple cumsum of numbers [0 .. 5793] already starts having a significant error due to precision.

numpy.float32(16776528 + 5793)
16782320.0
numpy.float64(16776528 + 5793)
16782321.0

It might be that other implementation (non Eigen) are smarter and do accumulation in 64 bit, so the error does not accumulate. Note you can use K.set_floatx('float64') to get the necessary precision

Nagging Assignee @azaks2: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.


Setting animation to false breaks line and scatter error bars

When I set animation: false on line and scatter charts, error bars aren't drawn.
To Reproduce

Visit the CodePen: https://codepen.io/mossymaker/pen/zYRmwPP2.
Toggle the animation option.

Expected behavior
Error bars render without animation.
Context

Version: chartjs@3, chartjs-chart-error-bars@3
Browser: Safari 15.5; also happens on Chrome

Additional context

I came across this issue due to using chartjs-node-canvas which sets animation to false automatically.  Scatter chart error bars aren't showing up, but bar chart error bars work.
Stepping through the code, it looks like the issue is that for scatter and line plots, chart.js has a "directUpdate" mode that gets used when animation is disabled.  In this mode, it does not call updateElement() on each element, which results in the custom properties e.g. xMin, xMax not getting transferred from the parsed data objects to the PointWithErrorBar objects.
If I manually disable directUpdate using a step debugger, I was able to get a chart to render with error bars.  However I can't figure out a way to prevent directUpdate from being set by changing any configuration.  Even if I alter the chartjs-node-canvas library to set animation: true, chart.js will override it, since it detects you aren't in a DOM platform.
So I'm not sure if this can be addressed by a change to the chartjs-chart-error-bars module.  I might try forking chart.js and adding an option to disable directUpdate, I think that would probably work.

I can confirm the error bars on scatter plots are working now in 3.8.0.  Thank you @sgratzl !!

Thanks, @sgratzl! ✨


Deprecations

Per the beta.18 release, typeKey should now be referenced as modelName and snapshot.constructor as snapshot.type
Currently it's still valid, but throws deprecation errors.

I'd be willing to make an attempt at fixing this. @slindberg do you want to try to do version detection for this one or just change the references?

I believe the only offending line is this one, since all other references to typeKey are for polymorphism support.
I think the fix can be as simple as
var modelName = snapshot.modelName || snapshot.typeKey;

return get(snapshot, 'record.store').serializerFor(modelName).serialize(snapshot);

@jakesjews if you want to open a PR, I'll give you a :raised_hands:

PR submitted. Your code suggestion did the trick.


support pegasus connect zookeeper which one use kerberos

Feature Request
Is your feature request related to a problem? Please describe:
In our business, pegasus cluster need to connect zookeeper which use kerberos protocol. I want support it.
Describe the feature you'd like:
Before version 2.3, we support it by customized zookeeper c client which convert java client use JNI. From version 3.7.0, zookeeper support c client use kerberos. I want to introduce it for pegasus.
Through my test, two parameter is required for 'zookeeper_init_sasl' different from 'zookeeper_init' which func get handle that represents a connection to the ZooKeeper service.

Current implementation

_handle = zookeeper_init(zookeeper_session_mgr::instance().zoo_hosts(),
                         global_watcher,
                         zookeeper_session_mgr::instance().timeout(),
                         nullptr,
                         this,
                         0);

Support Kerberos implementation

zoo_sasl_params_t sasl_params = { 0 };
sasl_params.service ="zookeeper"
sasl_params.mechlist = "GSSAPI";
_handle = zookeeper_init_sasl(zookeeper_session_mgr::instance().zoo_hosts(),
                              global_watcher,
                              zookeeper_session_mgr::instance().timeout(),
                              nullptr,
                              this,
                              0,
                              NULL,
                              &sasl_params);

When we have completed the above initialization and own valid kerberos ticket, we finised this job. How to get valid kerberos ticket already implemented on our code (kinit_context.cpp: https://github.com/XiaoMi/rdsn/blob/fc41809ce1622a47a535a2316df91d4d626f35ed/src/runtime/security/kinit_context.cpp).
Finally, we need to add some configuration items:

FLAGS_enable_zookeeper_kerberos
FLAGS_zookeeper_kerberos_service_name: It's always zookeeper, but it can change more

by the way，this issue is same as https://github.com/XiaoMi/rdsn/issues/85

It's a good idea for pegasus to adapt to various environments. can you pull a request for the function?

here: https://github.com/XiaoMi/rdsn/pull/979


Thought process behind why a lambda function value cannot be assigned to const variable?

I wrote a lambda function to generate repeated dashes and assign to a variable (I later understood this could be achieved with new String('=', 20) but thats another story).
Code looks like this.
public static string Dashes = ((Func<string>) (() =>{
            var dashes="";
            for (var i = 0; i < 20; i++)
            {
                dashes+="=";
            }
            return dashes;
        }))();

This works as expected but if I change public static string Dashes to public const string Dashes it will throw the error The expression being assigned must be constant
I understand if this error is thrown when the compiler thinks our value might vary (hence cannot be constant) but the lambda function here executes exactly once in program's lifetime... What is the thought process here for the error?
Just as a reference, the following code works perfectly fine in JS
const Dashes = () =>{
            var dashes="";
            for (var i = 0; i < 20; i++)
            {
                dashes+="=";
            }
            return dashes;
    };

Will C# also have this feature in the future? Or is there a deeper reason why it might be impossible?

The keyword const is a bit different in C# than in other languages. In C#, const requires the the compiler understands what the value is and that it never changes. And since the compiler is very limited in this regard, const fields are very limited in what you can assign to them. Instead, you should mark the field as readonly.

I hope the question is answered here, closing the issue.


Sandboxed decrypted styles/fonts not fully imported

On Chrome, there's no api I can find to get the CSS for different pseudo classes or to get all the fonts for the page, or for that matter read external stylesheets. It sadly looks like the solution is to do an ajax request from the background script (https://developer.chrome.com/extensions/xhr) for external stylesheets, and then parse them for fonts and styles applied to the elements inserted in the sandboxed decryption and then passed the parsed values into the relevant iframes. This seems so hacky

This issue was moved to GrdMe/GrdMe#40

This issue was moved to GrdMe/GrdMe#40


Accessibility Patch in CodeMirror

Hi
So I've created a hackish kind of patch for enabling screen reader to read the contents of editor. As far as I have checked this is working fine for me, at least for my purposes. Can someone check/test this out so that I can convert into a proper patch and raise a pull request after making necessary changes.
Thanks
Just include this in your source code:
https://gist.github.com/ce244ac29072c267bd18
Please note that if you insert an html element with aria-live tag dynamically, it won't be picked by some AT unless it is present in the source code at the time of first rendering.

Have you tried setting the option inputStyle: "contentEditable"? That should give you much  better results.

I tried it out but I was having problem with VoiceOver in Mac. On NVDA it was usable though.

Okay. But this patch doesn't really look like a viable direction. I'd much rather continue working out problems with the contenteditable input style, which provides the browser with much more information (and thus probably requires less invasive hacks).


Handle connection timeout

Purpose
Handle connection time-out.
Background
We use expiring connection for a better control of resources. The expiration time is a compile-time constant. It is set to be 10 minutes, which was sufficient until now. We encountered a case of deleting 10M nodes that takes 10+ minues. While waiting for the results, the client loses the connection to the server. The connection closedown is initiated by the server.
Solution
When a IO call returns prematurely, we check that there are no errors at (i) SSL level and (ii) system lib level (except for nominal EAGAIN that is used to indicated expiration), and we retry the operation.
Issues expected to be resolved
Fixes https://github.com/aperture-data/athena/issues/416
Next step (to be done as a separate PR)
IO error handling is incomplete. E.g. this package can close a connection without notifying the client; the client will only learn that the connection is closed when it tries to write to the socket.

Possibly fixes https://github.com/aperture-data/athena/issues/425


Identical types reported as not matching

I'm getting a type warning comparing two types which are identical. The type is somewhat complex, using generics and unions. I suspect maybe it has something to do with a subtle difference in the way the two places were defined, e.g. (Foo?)= vs (Foo|null|undefined), but I don't know. Here's the warning:
./foo.js.tmp:3585: WARNING - actual parameter 1 of callback does not match formal parameter
found   : (Result<{image: (HTMLCanvasElement|HTMLImageElement|null|undefined), status: string}>|null)
required: (Result<{image: (HTMLCanvasElement|HTMLImageElement|null|undefined), status: string}>|null)
		callback(Result.ok({image: pageCacheEntry.image, status: 'done'}));

This occurred with the following version:
Version: 1.0-SNAPSHOT
Built on: 2015-11-24 22:30

I tried to get a more recent build just in case it's already been fixed, but the current build fails due to some internal dependency issue.

There is some silly distinction in the type checker between inferred and declared record types. I'm guessing that distinction is what causes this bug. Workaround: declare the type of the object literal passed to Result.ok, either with a cast, or before the call.

(Marking as low priority as we're mostly working on the new type checker these days.)


Concern about version of Tensorflow and python

In the basic usage, you say that
Weight Conversion from the Original Repo (Requires TensorFlow 1.x)
TensorFlow 1.x can only installed for python2.
However the code in test.py is using python3 syntax.
How did you manage to make it work?

I got it, it is actually tensorflow-gpu 1.15.0, not tensorflow. The require version of python is 3.6
You should really state it more clearly in README.
I figured it out by digging to original repo.

Can you edit the readme maybe?
Im trying since hours to get this to work.
regards
lilly

Got it, thanks.


nodejs template: kebab-case component schema name creates invalid JavaScript

e.g.
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/foo-bar-baz'

The generate API files have code like this:

module.exports.do_foo = function do_foo (req, res, next) {
  var foo-bar-baz = req.swagger.params['foo-bar-baz'].value;
  // ...
}

- is not a valid identifier character in JavaScript, because it's used as a mathematic operator foo - bar - baz.
The identifier is also used in the generated impl/API service:
exports.do_foo = function(foo-bar-baz) {
  // ...
}

Note: look at interaction of variableNamingConvention and modelPropertyNaming and see if one should affect parameter naming.