ENH: make apply_function aware of channel index

Fixes #10034.
this PR makes the index of currently processed channel available within custom functions applied to data.
this is needed for processing channels differently during parallel processing, as requested in #10034 .

uses inspect.getfullargspec to check the custom function for input arguments.
currently I only allow a specific variable name (ch_idx) to be detected - please suggest an alternative naming, if appropriate.
alternatively, one could just check for number of requested variables, but i think this might make things messier.
initial commit only for Base.raw. if you are happy with the solution i will copy to epochs and evoked

I can also extend the code to support ch_name.

quick question for you @drammock :
when porting the functionality to evoked arrays I noticed that Evoked.apply_function did not currently have the option to process the whole array in place (-> channel_wise=False).
discussing with @wmvanvliet and @britta-wstnr we couldnt think of a good reason why this should be intentional, so I implemented this functionality analogous to Rawand Epochs.
I did so in a separate commit (https://github.com/mne-tools/mne-python/pull/12206/commits/f88ccd0e4e99b655764a8a43e6adb10eb22bb007), so no problem to revert if there was a good reason..

I like the functionality you're adding here!
What I found a little surprising, though, is that we allow functions that by themselves accept parameters passed by position, but then we insist on specific parameter names. I have never seen this anywhere in any other Python project I've worked on. If we want to enforce a parameter name, we should probably insist on a keyword-only argument

we should probably insist on a keyword-only argument

We sort of do that already, since the parameters need to have a certain name and we pass them through **kwargs

we should probably insist on a keyword-only argument

We sort of do that already, since the parameters need to have a certain name and we pass them through **kwargs

Ah okay, then I misunderstood the implementation! I didn't have a chance to run the code yet
Thanks for the clarification

we should probably insist on a keyword-only argument

We sort of do that already, since the parameters need to have a certain name and we pass them through **kwargs

Ah okay, then I misunderstood the implementation! I didn't have a chance to run the code yet
Thanks for the clarification

Hey, I thought about this again and I'm still having concerns. I'm however traveling today and may only be able to write an explanation of my thoughts sometime later tonight

I think I know what @hoechenberger means; why do we allow both positional and keyword arguments? Wouldn't it be easier and safer to enforce e.g. positional-only? Currently, inspect.getfullargspec() is used (BTW, since it returns a named tuple, I'd access the return value via names and not positions 0 and 4), which I don't think is necessary if we only allowed positional-only args. Callback functions should use a f(x, ch_idx, ch_name) signature (to create some sort of consistency), but currently, ch_idx and ch_name can be passed in any position and both positional and via keyword.
If channel_wise=True, we could also enforce callbacks with that signature even if they don't use ch_idx or ch_name (they can just discard them) to make things simpler.

I believe @cbrnr orerry much captures my thoughts

IMO, these should all be allowed from the user's side:
f(x)
f(foo)
f(x, ch_idx)
f(x, ch_name)
f(x, ch_idx, ch_name)
f(x, ch_name, ch_idx)
f(x, *, ch_idx)
f(x, *, ch_name)
f(x, *, ch_idx, ch_name)
f(x, *, ch_name, ch_idx)

which simply means MNE-Python will always call f by passing x by position and ch_name/ch_idx always by name. This way, there is the fewest way for the user to make a mistake I think.

f(x, ch_idx)
f(x, ch_name)
f(x, ch_idx, ch_name)
f(x, ch_name, ch_idx)

I disagree quite strongly with this. You're allowing for positional parameters but determine their functionality based on their name. I have never seen this anywhere else. It's too much magic. Either accept parameters by position, and then the meaning of the parameter is determined by its position, or do kw-only args and then the parameter name determines its function. I don't like the hybrid approach implemented here, I still feel it's an antipattern

f(foo)

What does this do? The first argument must always be the data array, or no?
f(x, ch_idx)
f(x, ch_name)
f(x, ch_idx, ch_name)
f(x, ch_name, ch_idx)

These should not be allowed. Why do you want to mis-use positional arguments to act like keyword arguments?
f(x, *, ch_idx)
f(x, *, ch_name)
f(x, *, ch_idx, ch_name)
f(x, *, ch_name, ch_idx)

That's fine if you prefer kw-args over positional args.

i totally see your point, @hoechenberger @cbrnr . definitely weird

IMO, these should all be allowed from the user's side:
f(x)
f(foo)
f(x, ch_idx)
f(x, ch_name)
f(x, ch_idx, ch_name)
f(x, ch_name, ch_idx)
f(x, *, ch_idx)
f(x, *, ch_name)
f(x, *, ch_idx, ch_name)
f(x, *, ch_name, ch_idx)

which simply means MNE-Python will always call f by passing x by position and ch_name/ch_idx always by name. This way, there is the fewest ways for the user to make a mistake I think.
These are not allowed:
f(x, foo)
f(x, foo, bar)

what @wmvanvliet wrote is the line along which i decided for this implementation - especially the last point led to this kind of weird design: besides the data users can only request ch_name and/or ch_idx in their custom defs but nothing else.
i dont really see how I could do this in a better way. any good idea?

I would simply not allow any other signatures apart from f(x, ch_idx, ch_name). If a user opts into channel_wise=True, their callback must match exactly this signature. Even if they don't need one of the two (or both), it is not a problem to define the function like this.

but this is not really backward compatible, right?
channel_wise=True is the default, and users so far had to provide functions with a single argument

What about introducing one or two new parameters in .apply_function() then? E.g. pass_ch_idx=False and pass_ch_name=False? That way, the default callback will still be f(x), but it can be customized if so desired.

e.g. pass_ch_idx=False and pass_ch_name=False

That's sounds worse from a user perspective.

If it's one: behave like we always used to
Two: data and channel index
Three: data, channel index, and name

I can live with that.

You're allowing for positional parameters but determine their functionality based on their name. I have never seen this anywhere else

I don't understand this comment, sorry. Python does not have a strict distinction between positional and named parameters. If you have a function signature like f(x, ch_idx, ch_name) you can still call it like this: f(foo, ch_idx=1, ch_name="channel1"). Here, x is called positional and ch_idx and ch_name are called by name. I simply suggest always calling ch_idx and ch_name by name when we call the user-supplied function. That way, even if the user wrote their function signature like f(x, ch_name, ch_idx) it doesn't matter. This is a very standard way of calling a python function, one which we use extensively across the MNE-Python codebase whenever we have functions that take a lot of parameters, or am I missing something here?

That's sounds worse from a user perspective.

Why? The defaults are False and as @dominikwelke said:

as a side note, i guess very few users will want to use this functionality..

That's sounds worse from a user perspective.

Why? The defaults are False and as @dominikwelke said:

Because I find clearer from a user perspective to provide a function with a signature f(x, ch_name) rather than f(x) which will have it's signature silently modified to f(x, ch_names) because of this extra argument + now f(x) will have some undefined ch_name variable in its scope.

I don't understand this comment, sorry.

There are two major problems:

you allow for a function signature that has positional parameters, but you force users to adopt a specific name for their positional parameters. This is highly unusual
using the same function signature but with a different name of the parameter changes function behavior. This, again, is something I have never seen done anywhere and it would cause a great deal to confusion to me if I couldn't name my positional parameters the way I want to

The solution is simple:
Either expect positional arguments and pass values by position, or enforce kw-only args and pass by name.

I mean seriously, if we have to use inspection to find out how to call a function, doesn't it sound like a hack to you immediately?

you allow for a function signature that has positional parameters, but you force users to adopt a specific name for their positional parameters. This is highly unusual

Would you be OK if we force the signature to be one of:
f(x)
f(x, *, ch_name)
f(x, *, ch_id)
f(x, *, ch_name, ch_id)
f(x, *, ch_id, ch_name)

using the same function signature but with a different name of the parameter changes function behavior. This, again, is something I have never seen done anywhere and it would cause a great deal of confusion to me if I couldn't name my positional parameters the way I want to

I'm not sure I follow this one, anyway the function signature can not be different from the one mentionned above. f(x, foo) would raise, I think?

Because I find clearer from a user perspective to provide a function with a signature f(x, ch_name) rather than f(x) which will have its signature silently modified to f(x, ch_name) because of this extra argument + now f(x) will have some undefined ch_name variable in its scope.

I'm not sure we're talking about the same thing. I'm not proposing to silently modify the function signature.

f(x)
f(x, *, ch_name)
f(x, *, ch_id)
f(x, *, ch_name, ch_id)
f(x, *, ch_id, ch_name)

Yes, this would work if we want to go with kw-args
Personally I'd prefer to pass by position but the above would be acceptable for me too

... would still require inspection though ... but I think it keeps it to minimum at least
I'd prefer to have a solution that can do without any inspection ideally
I can see why you don't like @cbrnr's proposal so much though
OTOH this is how we've been doing things in MNE-Python in many places for many years
So I don't understand why this is immediately off the table now?
I'd love to hear @agramfort weigh in if he has time

Also I'd like to say: thanks for everyone contributing here, be it code or by sharing their opinion on this discussion! 🤗

If we really want to do multiple dispatch in Python, why not use the multipledispatch package (pure Python, no deps) like this:
import numpy as np

from multipledispatch import dispatch

@dispatch(np.ndarray)
def f(x):
    print(f"Got only one argument")

@dispatch(np.ndarray, int)
def f(x, ch_index):
    print(f"Got two arguments")

@dispatch(np.ndarray, int, str)
def f(x, ch_index, ch_name):
    print(f"Got three arguments")

Other variants could be added (with swapped int and str parameters for example.

After a discussion with @britta-wstnr, @larsoner and @dominikwelke we are all in favor of the current implementation (as is I think @wmvanvliet and @mscheltienne also are?).  Here is the rationale as I see it:

currently, we impose quite minimal constraints on the user-passed function: it must take 1 argument (can be called anything: x, foo, whatever) that is a NumPy array, and return a NumPy array of the same dimensions.
if we were only adding 1 more supported parameter, we could say "interpreted positionally, call it whatever you want". But because we're adding 2 more supported parameters, we must either:

force an order on the new params which means that if ch_idx comes before ch_name, then ch_idx must always be in the signature even if the user only wants/needs to use ch_name. That is slightly burdensome on the user.
allow free order on the new params which means that we must either:

force the user to make them kw-only, which means explaining what the * in the function signature means, and forcing them to use it. This is also slightly burdensome on the user.
allow the user to make the params technically positional and in either order, and we inspect the signature to see what params are there, and pass in (by name) one or both of the two extra params that we're offering to supply. This boils down to (as @hoechenberger said)

allowing for positional parameters but determine their functionality based on their name.

I agree that this is unusual. But to me the question from the UX perspective is: do we compel the user to write their one-off channel-modifying function to follow best-practices around APIs?  Or do we say "its first arg must be the data, and it can have 1 or 2 others (ch_name and/or ch_idx) if you want/need them." To me this is a perfect use case for inspect --- it allows us to make the user's job slightly easier, reduces the amount of explanation we have to give around how to craft these functions, and as long as they follow the instructions about param naming they don't need to think/worry about param order.

I am still not in favor, but whatever.

I see your arguments but I disagree that this makes it easier for users. Like you said, it's unusual, hence likely to cause confusion or at least additional cognitive load. It sets MNE's behavior apart from the rest of the Python ecosystem by introducing some kind of meta language feature, and thereby introduces inconsistencies without any need to do so.
My 2 ct

This is not a veto. But I want it to be noted that I think this implementation is a bad idea.

Like you said, it's unusual, hence likely to cause confusion or at least additional cognitive load.

For transparency, my perspective rests on the assumptions that

it seems unusual only because we have intermediate/advanced knowledge of how Python works
many/most of our users are not Python experts, and will not even notice that there is anything unusual about this at all, because they are not calling their own function (we are calling it behind the scenes).

Personally I had been coding in Python a few years before even seeing an * in a function signature, and then I had to look up what it meant. Should we require users have that bit of background knowledge, if we can safely do what needs doing without requiring it?

Thanks for the meeting summaries and explanations, @drammock
I know it's difficult to keep everyone involved & informed here while we're working and discussing so asynchronously. I appreciate your effort.

@dominikwelke I think you should be okay to proceed with the inspect-based implementation

as soon as users start gaining experience and using other libraries like scikit-learn, they'll experience confusion due to this inconsistency.

I think this is an assumption without evidence. They might not notice or remember the difference, they might be confused, or they might think (with curiosity or even admiration) "huh, I wonder how MNE-Python pulled that off?"

We don't do this stuff anywhere else even within MNE

Depends on what you mean by "this stuff". We do use inspect elsewhere. I don't think we take this specific approach to callables elsewhere, but we've also AFAIK never needed to (i.e., never had a motivation to be flexible about the call signature of a passed-in callable). Now we have that motivation. FWIW I don't think we should change our whole codebase to do this for all callables, it wouldn't make sense.

We're asking users to define their own function that does NumPy data manipulation but don't trust they'll be able to write a function signature or know any Python basics?

to me this isn't about trusting the user, it's about making life easier for them. We do this all over the place by allowing list/tuple/array, for example, or by allowing them to pass "mean" instead of passing the actual function np.mean. Here, we're saving them the trouble of writing the extra *, in their callable, or even knowing about how to specify a keyword-only param.

kw-only arguments are relatively new and advanced stuff.

Wait, are they a "Python basic" or are they "advanced stuff"?

but when it comes to functions, we start our very own special thing because MNE-Python is so special.

Let me give an analogy. I want to use a function that someone else wrote. I see it has a signature def divide(foo, bar):. Of course I read the docstring, because I don't know if I'm supposed to pass dividend, divisor or the other way around.  If instead the signature was def divide(*, foo, bar): that doesn't help me at all because I still need to know what to pass in as foo and what to pass in as bar. Now instead imagine the signature was def divide(dividend, divisor):. Great! Even if there's no docstring, I feel fairly safe writing assert divide(6, 2) == 3 (positional) or assert divide(divisor=2, dividend=6) == 3 (keyword).
Now turn the perspective around. The "user" in this case is not me, it is MNE-Python's code. It needs to call a function that someone else wrote. It cannot read that function's docstring, and it cannot (easily) read that function's source code to figure out how it works. But what can it do? It can look at the param names, and say "aha! I have a dividend and a divisor here, and I see param names dividend and divisor in the function signature. I feel confident that I can pass dividend=dividend, divisor=divisor in either order and expect to get the right result regardless of the order they appeared in the function signature and regardless of whether the function would have accepted them as positional!"
To me that is a case of our code handling realistic, real-world user input (callables) flexibly, smartly and gracefully. It is not us "starting our very own special thing".
With that, I'm going to sign off of this discussion. My goal here is not necessarily to change your vote @hoechenberger, but hopefully lead you to feel less vehemently opposed to the will of the majority. You're welcome to have the last word for posterity, if you want it.

You're entirely ignoring the possibility of passing parameters by position, or @cbrnr's proposal of adding a switch to control the behavior, like we do elsewhere. I don't know why you're providing an example of good vs bad parameter naming, it just seems out of place.

Given that this is a pretty niche bit of functionality, could we perhaps YAGNI a bit and only give the channel index, not its name? So either f(x) or f(x, ch_index). Given that the purpose of the function is to modify the data (as opposed to say plotting it) I think ch_index will be used a lot more than ch_name. Perhaps it's not too much to ask for the user to do a inst.ch_names[ch_index] to look up the channel name if they need to. Or is there some scenario in which it would be harder to look up the channel name? (dropped bad channels come to mind).

just applied @mscheltienne 's suggestions. let me know what to do next

could we perhaps YAGNI a bit and only give the channel index, not its name? (...) Or is there some scenario in which it would be harder to look up the channel name? (dropped bad channels come to mind).

this would work, but yes -  for the user it would be more dificult if they wanted to use ch_names. they would have to pull/create a list or dict with the channel names before or while setting up their def that must correspond to insts ch_names  when eventually processed. this seems doable but quite error prone and i think is incompatible with rejecting channels between def declaration and calling apply_function.
in the end we would require users to keep track of something that is readily available within apply_function's scope (->inst.info)
if this was to be the desired route, i would rather consider only offering ch_name - this way ppl could directly access relevant information (e.g. scalp location or hemispere in case of eeg) and define more transparent/human readable dictionaries if they needed an int.

I understand the decision has already been made, but for the record, I would also be fine with just having either 1 or 3 positional parameters. So either f(x) or f(x, ch_idx, ch_name). The inconvenience to the user to have a possibly useless parameter passed to their function is so small it doesn't warrant forcing through an apparently controversial design decision.

hi @drammock and all
apologies it took me forever to look into this again..
i applied the requested changes


Added Import Options

Added import functionality for:
Visual Code Grepper (An Open Source static source code analysis tool) XML and CSV output
OWASP Dependency Check XML output
Generic Findings CSV file

man, @jpbowie, great work! I've reviewed and everything looks good to me, but I'll wait for @grendel513 and @aaronweaver to weigh in.

@devGregA, @jpbowie Looks good! +1 on the unit tests as well. Would you also happen to have the sample files as well? If so could you add them here? https://github.com/OWASP/django-DefectDojo/tree/master/docker/sample_data/scans

@devGregA, @jpbowie - taking a look now.

@devGregA, @aaronweaver, @jpbowie - Thanks for all this work @jpbowie, the more tools that are integrated the greater the value Dojo provides.  Here are my notes/thoughts:

I don's see where these templates are used: add_dependency_check_scan.html, add_generic_import.html, add_vcg_scan.html - looks like they can be omitted from the pull request.
The file dojo/help/add_generic_import_instructions.html referenced in the template dojo/templates/dojo/import_scan_results.html does not seem to exist.  Additionally, I would recommend this be added to the readthedocs rst file (https://github.com/OWASP/django-DefectDojo/blob/master/docs/features.rst#imports) where all imports are already slightly documented.  A new page can be added for the CVS format, etc...but it should be part of readthedocs, and then linked to from the template to keep all documentation in a central location.  The help directory does not exists now and I don't feel there is a need for it since we leverage readthedocs.  The readthedocs documentation will auto generate when the changes are committed and merged into the branches.

If you can address these we can merge the pull request right away.
Great job on including tests!

Thanks, I'll make these changes and clean up a few other things with the generic file import that have been bothering me.

I think I've made all of the changes and committed them.  Let me know if there's anything I missed.  Thanks.

Hi @jpbowie great work! I've been pushing to try to get the word out about Dojo in the hopes that we could get more awesome contributors like yourself. If you don't mind me asking how did you find out about Dojo?

I was at AppSecUSA 2016 and attended the "Creating and Automating your own AppSec Pipeline" course by Matt Tesauro where we used DefectDojo in some of our labs.  I had been evaluating another product, but after the class I thought I'd try DefectDojo, make a few customizations,  and hopefully contribute back to the community.


Chris, thanks so much for running it on all those projects! It is abundantly clear which ones have linting enabled in CI and which don't...
For display I think making people import it is a good idea. I know I was very confused the first time I saw a reference to it in a notebook and couldn't find the import.
Yep I agree. I actually think #noqa with an explanation for hvplot imports isn't such a terrible idea. It makes it clear what is going on and these are all examples so they should be explicit.
I've been seeing the escape sequence warning cropping up in various places, but hadn't looked into it yet.

About the invalid escape sequence warnings: their appearance is probably down to whatever warning filter is used by pytest, but it's not an nbsmoke or a pytest thing:
> set PYTHONWARNINGS=default

>type esc.py
"\z"

>python esc.py
esc.py:1: DeprecationWarning: invalid escape sequence \z
  "\z"

>type esc2.py
r"\z"

>python esc2.py

> python --version
Python 3.6.8

Changed in version 3.6: Unrecognized escape sequences produce a DeprecationWarning. In some future version of Python they will be a SyntaxError.

https://docs.python.org/3/reference/lexical_analysis.html

Ok, sounds like those warnings are real and we should be adding escape characters to the strings in those notebooks. Thanks.
I'm happy with either the whitelist or the #noqa approach for hvplot; I'll leave that up to you two.

I've fixed the flake checking under python 2 for the benefit of charities etc that can't afford to upgrade from python 2. Still got the appveyor/windows tests to fix (I'm using windows locally, so surprised they don't just pass..). Also still got the conda tests to fix (I can't run conda locally - laptop too slow :( - but I should be able to figure it out from the travis logs later).

Finally, the travis tests are passing.
When you run nbsmoke's tests with current versions of jupyter ecosystem packages, you get quite a few deprecation warnings (e.g. from how nbconvert uses ipython and traitlets). nbsmoke's tests are counting/checking warnings during the tests, to ensure warnings are not being generated or are being generated appropriately for each test. I want to filter out the deprecation warnings (i.e. completely ignore them), but the only way I've found to do that is by setting the PYTHONWARNINGS environment variable. Setting pytest's own warnings filter results in some (though not all) deprecation warnings still appearing. I suppose I could modify nbsmoke's warning checking to filter out deprecation warnings, but that looks a bit tricky to do (maybe if I knew pytest better it would be easy?). Requiring an environment variable to be set for the tests to pass is a bit annoying for nbsmoke developers, nbsmoke packagers, and users of nbsmoke who want to run its self tests after installing.
Anyway, for the python/pip tests, I set PYTHONWARNINGS in tox.ini, and that seems ok: even though it's annoying to have to set an environment variable, at least doing it there will mean it happens for anyone running the tests via tox.
However, I don't know how to do the same for conda. In the end, I set PYTHONWARNINGS on travis and then instructed conda build to pass that through. Maybe there's a better way?

Good question. I don't know of one.

I jsut opened #21, #22, #23, #24, and #25 - happy to get comments on any of them (even if it's just "I don't understand the point" :) ).
I'll do #23 as part of this PR because I have looked at the code recently and only involves nbsmoke itself (not e.g. pytest) so it will be easy. And I'll remove the hv opts handling plus fix the appveyor tests. At that point, I think it should leave something that will work as desired for current pyviz projects as they are now (having to use noqa for hvplot, though) - I'll verify that by running over all pyviz projects again.

I've removed the need to set the PYTHONWARNINGS environment variable.
(Testing plugins is a bit different from normal testing; I've now re-learned what I apparantly used to know when I initially created the tests: https://docs.pytest.org/en/latest/writing_plugins.html#testing-plugins. I realized that options passed into pytest at the top level weren't applied where needed, because the tests are running pytest themselves (since nbsmoke is a pytest plugin). The problem was that each test's "parseoutcomes()" was picking up warnings which the "top level" pytest was set to ignore.)

I guess the appveyor tests have been lost in one of the pyviz renames? :(

Running again over some of our projects...
geoviews
(nbsdev) D:\code\ioam\geoviews>pytest -v --nbsmoke-lint --nbsmoke-lint-debug examples
=============================================================== test session starts ================================================================
platform win32 -- Python 3.6.8, pytest-4.4.1, py-1.8.0, pluggy-0.9.0 -- d:\venvs\nbsdev\scripts\python.exe
cachedir: .pytest_cache
rootdir: D:\code\ioam\geoviews, inifile: tox.ini
plugins: cov-2.7.1, nbsmoke-0.2.8.post44+gbdfd485
collected 35 items / 68 skipped

examples/Homepage.ipynb::D:\code\ioam\geoviews\examples\Homepage.ipynb PASSED                                                                 [  2%]
examples/gallery/bokeh/airport_graph.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\airport_graph.ipynb PASSED                           [  5%]
examples/gallery/bokeh/brexit_choropleth.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\brexit_choropleth.ipynb PASSED                   [  8%]
examples/gallery/bokeh/city_populations_2050.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\city_populations_2050.ipynb FAILED           [ 11%]
examples/gallery/bokeh/filled_contours.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\filled_contours.ipynb FAILED                       [ 14%]
examples/gallery/bokeh/great_circle.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\great_circle.ipynb PASSED                             [ 17%]
examples/gallery/bokeh/katrina_track.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\katrina_track.ipynb PASSED                           [ 20%]
examples/gallery/bokeh/new_york_boroughs.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\new_york_boroughs.ipynb PASSED                   [ 22%]
examples/gallery/bokeh/orthographic_vectorfield.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\orthographic_vectorfield.ipynb PASSED     [ 25%]
examples/gallery/bokeh/tile_sources.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\tile_sources.ipynb PASSED                             [ 28%]
examples/gallery/bokeh/trimesh_uk.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\trimesh_uk.ipynb PASSED                                 [ 31%]
examples/gallery/bokeh/vectorfield_example.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\vectorfield_example.ipynb PASSED               [ 34%]
examples/gallery/bokeh/world_population.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\world_population.ipynb PASSED                     [ 37%]
examples/gallery/bokeh/xarray_image.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\xarray_image.ipynb PASSED                             [ 40%]
examples/gallery/bokeh/xarray_quadmesh.ipynb::D:\code\ioam\geoviews\examples\gallery\bokeh\xarray_quadmesh.ipynb PASSED                       [ 42%]
examples/gallery/matplotlib/airport_graph.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\airport_graph.ipynb PASSED                 [ 45%]
examples/gallery/matplotlib/brexit_choropleth.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\brexit_choropleth.ipynb PASSED         [ 48%]
examples/gallery/matplotlib/city_population.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\city_population.ipynb PASSED             [ 51%]
examples/gallery/matplotlib/filled_contours.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\filled_contours.ipynb PASSED             [ 54%]
examples/gallery/matplotlib/great_circle.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\great_circle.ipynb PASSED                   [ 57%]
examples/gallery/matplotlib/katrina_track.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\katrina_track.ipynb PASSED                 [ 60%]
examples/gallery/matplotlib/new_york_boroughs.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\new_york_boroughs.ipynb PASSED         [ 62%]
examples/gallery/matplotlib/orthographic_vectorfield.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\orthographic_vectorfield.ipynb PASSED [ 65%]
examples/gallery/matplotlib/tile_sources.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\tile_sources.ipynb PASSED                   [ 68%]
examples/gallery/matplotlib/trimesh_uk.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\trimesh_uk.ipynb PASSED                       [ 71%]
examples/gallery/matplotlib/vectorfield_example.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\vectorfield_example.ipynb PASSED     [ 74%]
examples/gallery/matplotlib/world_population.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\world_population.ipynb PASSED           [ 77%]
examples/gallery/matplotlib/xarray_image.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\xarray_image.ipynb PASSED                   [ 80%]
examples/gallery/matplotlib/xarray_quadmesh.ipynb::D:\code\ioam\geoviews\examples\gallery\matplotlib\xarray_quadmesh.ipynb PASSED             [ 82%]
examples/user_guide/Geometries.ipynb::D:\code\ioam\geoviews\examples\user_guide\Geometries.ipynb FAILED                                       [ 85%]
examples/user_guide/Gridded_Datasets_I.ipynb::D:\code\ioam\geoviews\examples\user_guide\Gridded_Datasets_I.ipynb PASSED                       [ 88%]
examples/user_guide/Gridded_Datasets_II.ipynb::D:\code\ioam\geoviews\examples\user_guide\Gridded_Datasets_II.ipynb PASSED                     [ 91%]
examples/user_guide/Projections.ipynb::D:\code\ioam\geoviews\examples\user_guide\Projections.ipynb PASSED                                     [ 94%]
examples/user_guide/Resampling_Grids.ipynb::D:\code\ioam\geoviews\examples\user_guide\Resampling_Grids.ipynb PASSED                           [ 97%]
examples/user_guide/Working_with_Bokeh.ipynb::D:\code\ioam\geoviews\examples\user_guide\Working_with_Bokeh.ipynb PASSED                       [100%]

===================================================================== FAILURES =====================================================================
___________________________________________________________________ test session ___________________________________________________________________
D:\code\ioam\geoviews\examples\gallery\bokeh\city_populations_2050.ipynb
** line 18 col 0: 'geoviews.dim' imported but unused
To see python source that was checked by pyflakes: D:\code\ioam\geoviews\examples\gallery\bokeh\city_populations_2050.nbsmoke-debug-postmagicprocess.py
To see python source before magics were handled by nbsmoke: D:\code\ioam\geoviews\examples\gallery\bokeh\city_populations_2050.nbsmoke-debug-premagicprocess.py
___________________________________________________________________ test session ___________________________________________________________________
D:\code\ioam\geoviews\examples\gallery\bokeh\filled_contours.ipynb
** line 15 col 0: 'holoviews as hv' imported but unused
To see python source that was checked by pyflakes: D:\code\ioam\geoviews\examples\gallery\bokeh\filled_contours.nbsmoke-debug-postmagicprocess.py
To see python source before magics were handled by nbsmoke: D:\code\ioam\geoviews\examples\gallery\bokeh\filled_contours.nbsmoke-debug-premagicprocess.py
___________________________________________________________________ test session ___________________________________________________________________
D:\code\ioam\geoviews\examples\user_guide\Geometries.ipynb
** line 16 col 0: 'numpy as np' imported but unused
To see python source that was checked by pyflakes: D:\code\ioam\geoviews\examples\user_guide\Geometries.nbsmoke-debug-postmagicprocess.py
To see python source before magics were handled by nbsmoke: D:\code\ioam\geoviews\examples\user_guide\Geometries.nbsmoke-debug-premagicprocess.py
================================================= 3 failed, 32 passed, 68 skipped in 74.74 seconds =================================================

I guess the appveyor tests have been lost in one of the pyviz renames? :( Although I am developing on win, the appveyor tests were failing when I last worked on this PR.

Hmm yeah I can look into that. I'm a little surprised they are failing since you are the only one who's been making changes...

I remember I was also surprised to find appveyor was failing for this PR since I was developing on windows, but it was definitely failing.
I recall now I could not immediately figure out why it was failing. That's part of the reason this PR stalled before. Maybe I should get it in before you restore appveyor? :)

That is fine by me :) We don't need it to run on windows.

@jsignell I'm sure I'd like to improve the code, but it would also be good to start trying to use this version before another couple of years go by ;) So I'll happily fix or clarify stuff in response to any comments you make, but otherwise I'm done with this PR.

@jsignell, thanks for taking the time to review! Can't have been pleasant ;)
Let me know if I missed anything.


Make ServiceContext.GetService take a factory function instead of a type

Here: https://github.com/rspeele/Rezoom/blob/00d4a070d7ece529d582335d03bb900402e566a0/src/Rezoom/Execution.fs#L147
Requiring this to be a type means that in order for the batch type to be initialized with a parameter, it must be represented by a sub-type.  Additionally, this could enable a helper function which allows for simple wrapping of all of this.  Is there any reason this couldn't be a constructor function instead?
Motivating Example:
open System
open Rezoom
open Rezoom.Execution

type OrgId = OrgId of Guid
    with static member MakeOrgId () = OrgId (Guid.NewGuid())

type UserId = UserId of Guid
    with static member MakeUserId () = UserId (Guid.NewGuid())

let org1Id = OrgId.MakeOrgId()
let org2Id = OrgId.MakeOrgId()

let orgs = dict [ org1Id, "Org 1"
                  org2Id, "Org 2" ]

let users = dict [ UserId.MakeUserId(), ("User 1", org1Id )
                   UserId.MakeUserId(), ("User 2", org1Id )
                   UserId.MakeUserId(), ("User 3", org1Id )
                   UserId.MakeUserId(), ("User 4", org1Id )
                   UserId.MakeUserId(), ("User 5", org1Id )
                   UserId.MakeUserId(), ("User 6", org2Id )
                   UserId.MakeUserId(), ("User 7", org2Id )
                   UserId.MakeUserId(), ("User 8", org2Id )
                   UserId.MakeUserId(), ("User 9", org2Id )
                   UserId.MakeUserId(), ("User 10", org2Id ) ]

let getOrg id = orgs.[id]
let getUser id = users.[id]

let getOrgs ids = ids |> Seq.map getOrg
let getUsers ids = ids |> Seq.map getUser

type StepBatch<'a, 'b>(runBatch: 'a seq -> 'b seq) =
    let userIds = ResizeArray<'a>()
    let batchResults = lazy (runBatch userIds |> Seq.toArray)

    member this.AddToBatch(i) =
        let index = userIds.Count
        userIds.Add(i)
        fun () -> batchResults.Value.[index]

let getCache<'a> () =
    { new CacheInfo() with
        override this.Category = box typeof<'a>
        override this.Identity = box "ignorefornow"
        override this.Cacheable = true
        override this.DependencyMask = BitMask.Zero
        override this.InvalidationMask = BitMask.Zero
    }

//can't do this

//type Errand<'a, 'b>(arg : UserId, factory: unit->StepBatch<'a, 'b>) =
//    inherit SynchronousErrand<'b>()
//
//    override this.CacheInfo = getCache<StepBatch<'a, 'b>> ()
//    override this.CacheArgument = box arg
//
//    override this.Prepare(context : ServiceContext) =
//        let batch = context.GetService<UserStepBatch>(factory)
//        batch.AddToBatch arg
//
//type UserErrand(arg: UserId) =
//    inherit Errand<UserId, string*OrgId>(arg, fun () -> StepBatch<UserId, string*OrgId>(getUsers) )
//
//type OrgErrand(arg: OrgId) =
//    inherit Errand<OrgId, string>(arg, fun () -> StepBatch<OrgId, string>(getOrgs))
//
// Can create as many of these as I want trivially now!

//have to do this instead

type UserStepBatch() = inherit StepBatch<UserId, string*OrgId>(getUsers)
type OrgStepBatch() = inherit StepBatch<OrgId, string>(getOrgs)

type UserErrand(arg : UserId) =
    inherit SynchronousErrand<string*OrgId>()

    override this.CacheInfo = getCache<UserStepBatch> ()
    override this.CacheArgument = box arg

    override this.Prepare(context : ServiceContext) =
        let batch = context.GetService<StepLocal<UserStepBatch>, UserStepBatch>()
        batch.AddToBatch arg

type OrgErrand(arg : OrgId) =
    inherit SynchronousErrand<string>()

    override this.CacheInfo = getCache<OrgStepBatch> ()
    override this.CacheArgument = box arg

    override this.Prepare(context : ServiceContext) =
        // Get the batch for this execution step.
        let batch = context.GetService<StepLocal<OrgStepBatch>, OrgStepBatch>()
        // Add ourselves to the batch, return the result getter.
        batch.AddToBatch arg

Hey, great question. This does mess with composability and makes it hard to write generalizations for common, nothing-special errand types. In particular it should be a lot easier than it is to convert any function 'a list -> 'b list Task to a function 'a -> 'b Errand.
I would like to redesign this and now would be an OK time to make breaking changes, since not many people are using Rezoom at all, and most of them are almost certainly using it indirectly through Rezoom.SQL instead of defining their own errand types.
Now, as to the reason it was written this way at all. Within an execution step, the first time you call context.GetService<FooFactory, Foo>(), the factory's CreateService method is called to build the service. The second time you call it, you get the same service instance, looked up in a dictionary by typeof<FooFactory>. Either way, you know that the instance you get was created by FooFactory().CreateService(context) so you should know what behavior to expect from it.
If we passed a factory function to context.GetService, then there's no longer a reliable key to cache that service on. Functions aren't reliably comparable for equality, and the service type might not be enough either. Contrived example (sorry it's late, can't think of a real one):

type CoolStringService(str : string) =
     member this.CoolString() = str

type CoolStringErrand(returnString : string) =
     inherit SynchronousErrand<string>()
     override this.CacheInfo = // ... whatever
     override this.CacheArgument = box returnString
     override this.Prepare(context) =
         // using hypothetical new API
         // if this caches the service based on typeof<CoolStringService>,
         // then we get the same instance on subsequent calls in the same step,
         // even if they have a different returnString!
         // but we *do* want to get the same instance on subsequent calls
         // that have the *same* returnString... otherwise we'd never batch anything
         // how to accomplish that without being able to equality-compare functions?
         let svc = context.GetStepLocalService<CoolStringService>(fun () -> CoolStringService(returnString))
         fun () -> svc.CoolString()

let cool x = Plan.ofErrand (CoolStringErrand(x))

plan {
      let! foo, bar = cool "hello", cool "world"
      return "uhoh" // both foo and bar will be "hello"
}

I think there is probably a good solution to this (and it's probably quite a distance away from the current design), but need to think about it.

Also, the current way to do what you're trying to accomplish is something like this.
I haven't tried this code so expect compilation errors but hopefully the gist comes across.
////////////////////////////////////////
// utility code

type StepBatch<'a, 'b>(runBatch: 'a seq -> 'b seq) =
    let userIds = ResizeArray<'a>()
    let batchResults = lazy (runBatch userIds |> Seq.toArray)

    member this.AddToBatch(i) =
        let index = userIds.Count
        userIds.Add(i)
        fun () -> batchResults.Value.[index]

// inherit from this to have a different factory type per request type
[<AbstractClass>]
type StepBatchFactory<'a, 'b>(runBatch : 'a seq -> 'b seq) =
    inherit ServiceFactory<StepBatch<'a, 'b>>()
    override this.CreateService(_) = StepBatch<_, _>(runBatch)
    override this.DisposeService(_, _) = ()
    override this.ServiceLifetime = ServiceLifetime.StepLocal

// generic errand, just plug in the right factory type
type StepBatchErrand<'f, 'a, 'b when 'f :> ServiceFactory<StepBatch<'a, 'b>>
     (arg : 'a) =
     inherit SynchronousErrand<'b>()
     static let constCacheInfo =
         { new CacheInfo() with
              override this.Category = box typeof<StepBatch<obj, obj>>
              // only type of errand in its category so doesn't matter
              override this.Identity = box ""
              override this.Cacheable = true
              override this.DependencyMask = BitMask.Zero
              override this.InvalidationMask = BitMask.Zero
         }
     override this.CacheInfo = constCacheInfo
     // might want to require 'a is equality comparable
     override this.CacheArgument = box arg
     override this.Prepare(context) =
         let batch = context.GetService<'f, StepBatch<'a, 'b>>()
         batch.AddToBatch(arg)

////////////////////////////////////////
// now the part you repeat for whatever Foos and Bars

type UserStepBatchFactory() =
    inherit StepBatchFactory<_, _>(getUsers)

let getUser (userId : UserId) : (string * OrgId) Plan =
    StepBatchErrand<UserStepBatchFactory, _, _>(userId)
    |> Plan.ofErrand

type OrgStepBatchFactory() =
    inherit StepBatchFactory<_, _>(getOrgs)

let getOrg (orgId : OrgId) : string Plan =
    StepBatchErrand<OrgStepBatchFactory, _, _>(userId)
    |> Plan.ofErrand

Thanks for your detailed reply (sorry for the delay in responding, just got back from week-long trip)!
I managed to get my desired functionality using the current implementation with the following horrible code:
type FlexibleStepLocal<'a, 'b, 'c>(factory: unit ->'a) =
    inherit ServiceFactory<'a>()

//lots of code omitted here

type FlexibleServiceContext(config : IServiceConfig) =
    inherit ServiceContext()

//lots of code omitted here

    member this.GetService2<'a, 'b, 'c>(getFactory: unit -> FlexibleStepLocal<'a, 'b, 'c>) =
        let ty = typeof<FlexibleStepLocal<'a, 'b, 'c>>
        let succ, service = services.TryGetValue(ty)
        if succ then Unchecked.unbox service else
        let factory = getFactory()
        let service = factory.CreateService(this)
        let stack =
            match factory.ServiceLifetime with
            | ServiceLifetime.ExecutionLocal -> globals
            | ServiceLifetime.StepLocal -> locals
            | other -> failwithf "Unknown service lifetime: %O" other
        services.Add(ty, box service)
        stack.Push(fun state ->
            factory.DisposeService(state, service)
            ignore <| services.Remove(ty))
        service

//lots of code omitted here

type MyErrand<'a, 'b>(arg : 'a, factory: unit->FlexibleStepLocal<StepBatch<'a, 'b>, 'a, 'b>) =
    inherit SynchronousErrand<'b>()

//lots of code omitted here

    override this.Prepare(context : ServiceContext) : unit->'b =
        let context = context :?> FlexibleServiceContext
        let batch: StepBatch<'a,'b> = context.GetService2<StepBatch<'a, 'b>, 'a, 'b>(factory)
        batch.AddToBatch arg

let getErrand batchFunction arg =
    MyErrand(arg, (fun () -> FlexibleStepLocal(fun () -> StepBatch batchFunction)))

//repeat this bit
let retrieveUser id = getErrand getUsers id |> Plan.ofErrand
let retrieveOrg id = getErrand getOrgs id |> Plan.ofErrand

Are you open to PRs implementing a static method with optional parameters?


ci: include detekt with GitHub code security analysis

This PR includes detekt as a linter in the project. In contrast to ktlint, detect is more focused on coding problems and might provide some valuable hints. I have disable failing the build when detekt issues exist due to the current amount of violations that already exist in the current code base.
Detekt results are passed into the GitHub Code Scanning feature. Effectively this means that PRs will receive inline annotations for detected problems and we can use these as a hint when reviewing PRs.

There's still something wrong with the reported file paths. Trying to fix that.

Now paths are correct

:tada: This PR is included in version 3.4.1 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


Add hostentries to pass --add-host flags to the docker container

I need the ability to pass --add-host flags to the docker init script - this does that.

@gtmtech thanks for adding this, much appreciated.


Reorg typescript library

Codecov Report

Merging #41 into development will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           development      #41   +/-   ##
============================================
  Coverage        83.18%   83.18%
============================================
  Files               77       77
  Lines             1225     1225
  Branches           108      108
============================================
  Hits              1019     1019
  Misses             193      193
  Partials            13       13

Impacted Files
Coverage Δ

packages/element-core/src/ElementWallet.ts
100% <ø> (ø)

packages/element-core/src/ElementDIDManager.ts
90.47% <ø> (ø)

packages/element-core/src/MnemonicKeySystem.ts
100% <ø> (ø)

Continue to review full report at Codecov.

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


Test app assigned audio

Assumes an audio controller.
The key purpose is to verify that a reboot (sudo shutdown -r +1) from the guest doesn’t disrupt the set of assigned audio.
Test checking list of devices before and after reboot
Signed-off-by: Ruslan Dautov<EMAIL_ADDRESS>

Added
[device "sound"]
  driver = "ich9-intel-hda"
  bus = "pcie.0"
  addr = "1b.0"

passed info via
eden controller edge-node get-config --file $CNF

jq '.deviceIoList += [{ "ptype": "PhyIoAudio", "phylabel": "Audio", "phyaddrs": { "PciLong": "0000:00:1b.0" }, "logicallabel": "Audio", "assigngrp": "Audio", "usage": "PhyIoUsageNone", "usagePolicy": { "freeUplink": false, "fallBackPriority": 0 }, "cbattr": {} }]' < $CNF | \
jq '.apps = (.apps | map(if .displayname == "'$1'" then . + {"adapters":[{"type": "PhyIoAudio", "name": "Audio"}]} else del(.adapters) end))' | \
jq '.apps = (.apps | map(if .displayname == "'$1'" then . + {"purge": {"counter": 1}} else . end))' > $CNF.new
eden controller edge-node set-config --file $CNF.new

lspci before config
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:02.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:03.0 Communication controller: Red Hat, Inc. Virtio console
00:04.0 Unclassified device [0002]: Red Hat, Inc. Virtio filesystem
00:05.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:00.0 USB controller: Red Hat, Inc. QEMU XHCI Host Controller (rev 01)
02:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01)

lspci after configs 00:01.0 Audio device by configs should be on 00:1b.0
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 Audio device: Intel Corporation 82801I (ICH9 Family) HD Audio Controller (rev 03)
00:02.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:03.0 Communication controller: Red Hat, Inc. Virtio console
00:04.0 Unclassified device [0002]: Red Hat, Inc. Virtio filesystem
00:05.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:00.0 USB controller: Red Hat, Inc. QEMU XHCI Host Controller (rev 01)
02:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01)

After driver installation

aplay -l
aplay: device_list:274: no soundcards found...

arecord -l
arecord: device_list:274: no soundcards found...

Hi, @dautovri
I cannot find any drivers in your output loaded for 00:01.0
On Ubuntu VM you can try to run
sudo apt-get install linux-modules-extra-`uname -r`

and reboot you VM

@giggsoff Confirmed. On ubuntu VM this command will show a sound card in alsamixer.
sudo apt-get install -y linux-modules-extra-`uname -r` alsa-utils
sudo reboot

After that
aplay -L
    Discard all samples (playback) or generate zero samples (capture)
surround21:CARD=Intel
    2.1 Surround output to Front and Subwoofer speakers
surround40:CARD=Intel
    4.0 Surround output to Front and Rear speakers
surround41:CARD=Intel
    4.1 Surround output to Front, Rear and Subwoofer speakers
surround50:CARD=Intel
    5.0 Surround output to Front, Center and Rear speakers
surround51:CARD=Intel
    5.1 Surround output to Front, Center, Rear and Subwoofer speakers
surround71:CARD=Intel
    7.1 Surround output to Front, Center, Side, Rear and Woofer speakers

All steps need to be automated, including configuring qemu to enable the audio device.

@dautovri Seems test in Ubuntu's QEMU is not passing:
$ ./eden test tests/hardware_reboot/ -v debug
...
        # Audio device passthrough setup (0.296s)
        > exec -t 20m bash set_audio.sh eclient
        [stdout]
        time="2021-05-14T11:31:01+03:00" level=fatal msg="Cannot unmarshal config: proto:\u00a0(line 324:9): unknown field \"fallBackPriority\""
        [exit status 1]
        FAIL: testdata/hardware_audio_reboot.txt:20: command failure

--- FAIL: TestEdenScripts (0.00s)
    --- FAIL: TestEdenScripts/hardware_audio_reboot (273.19s)
FAIL
...
$ ssh -t -oServerAliveInterval=10 -oConnectTimeout=10 -oStrictHostKeyChecking=no -oPasswordAuthentication=no -i /home/sadov/work/EVE/github/dautovri/eden/dist/tests/eclient/image/cert/id_rsa -p 2223<EMAIL_ADDRESS>ubuntu@585cd58b-77d1-4489-84e7-369c417f9109:~$ lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Device 1234:1111 (rev 02)
00:02.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:03.0 Communication controller: Red Hat, Inc. Virtio console
00:04.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:05.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:00.0 USB controller: Red Hat, Inc. QEMU XHCI Host Controller (rev 01)
02:00.0 SCSI storage controller: Red Hat, Inc. Virtio block device (rev 01)
03:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01)
ubuntu@585cd58b-77d1-4489-84e7-369c417f9109:~$

Are you making any settings for the audio device in QEMU?
Test's log:
eden_test_hardware_audio_reboot.002.log

@sadov Yes, sure.  QEMU configuration file in PR.  https://github.com/lf-edge/eden/blob/2985d3169359106295c457d3db0ab20585fd078e/tests/eclient/qemu%2Baudio.sh
./qemu+audio.sh
./eden eve stop
./eden eve start
./eden test tests/hardware_reboot/ -v debug

@dautovri you need to add this step to the test scenario -- it's a bad idea to run something in tests by hands.

@sadov @dautovri I believe it's not ready for merge?

Yes. As far as I understand, we don't have an automatically working test right now.

EDEN_TEST_USB_PT=y EDEN_TEST=large ./eden test tests/workflow/ -v debug
To check automatically working test.

You mean EDEN_TEST_AUDIO_PT?

At this time large test failed on hardware_audio_reboot test:
$ make clean && make build-tests && EDEN_TEST_AUDIO_PT=y EDEN_TEST=large ./eden test tests/workflow/ -v debug
...
        FAIL: ../hardware_reboot/testdata/hardware_audio_reboot.txt:14: command failure

--- FAIL: TestEdenScripts (0.00s)
    --- FAIL: TestEdenScripts/hardware_audio_reboot (1270.10s)
FAIL

Full test log:
eden_test_audio.003.log

@sadov  Looks like timewait is not enough for app deployment.  In the latest commit, I increased the time and these tests are passing locally.

@dautovri local test on Ubuntu 20.04 passed:
$ make clean && make build-tests && EDEN_TEST_AUDIO_PT=y EDEN_TEST=large ./eden test tests/workflow/ -v debug
...
--- PASS: TestEdenScripts (0.00s)
    --- PASS: TestEdenScripts/hardware_audio_reboot (981.96s)
PASS


Added metadata for empty query results and extended API

1.) Made column metadata information accessible for empty query results at result event end.
2.) Added metadata within API description.
3.) Put information about info object at correct event.

FWIW this is now incorporated in the rewrite branch.


Reduce variable scope, make formatting consistent with surrounding code

This too wide scope minor defect was found with Cppcheck

:+1:


Segfault in timeout_markstart

https://github.com/CorsixTH/CorsixTH/issues/1091 was posted in CorsixTH today that looks to originate in luasocket (as packaged for Debian, awaiting confirmation on the version).
CorsixTH Code:
https://github.com/CorsixTH/CorsixTH/blob/b6fc9f9e0c6893720200088d21da4e2e14f0d319/CorsixTH/Lua/app.lua#L1448
Stacktrace:
#0  0x00007fffd40e01e1 in timeout_markstart () from /usr/lib/x86_64-linux-gnu/lua/5.2/socket/core.so
#1  0x00007fffd40e0599 in buffer_meth_send () from /usr/lib/x86_64-linux-gnu/lua/5.2/socket/core.so
#2  0x00007ffff7688c4d in luaD_precall (L=L@entry=0x745b380, func=<optimized out>, func@entry=0x7392af0, nresults=nresults@entry=-1) at ldo.c:319
#3  0x00007ffff769483d in luaV_execute (L=L@entry=0x745b380) at lvm.c:709
#4  0x00007ffff7688fc0 in luaD_call (L=L@entry=0x745b380, func=<optimized out>, nResults=nResults@entry=-1, allowyield=allowyield@entry=1) at ldo.c:402
#5  0x00007ffff768516d in lua_pcallk (L=0x745b380, nargs=<optimized out>, nresults=-1, errfunc=<optimized out>, ctx=<optimized out>, k=<optimized out>) at lapi.c:962
#6  0x00007fffd40e479f in ?? () from /usr/lib/x86_64-linux-gnu/lua/5.2/socket/core.so
#7  0x00007ffff7688c4d in luaD_precall (L=L@entry=0x745b380, func=<optimized out>, func@entry=0x73929d0, nresults=nresults@entry=3) at ldo.c:319
#8  0x00007ffff769483d in luaV_execute (L=L@entry=0x745b380) at lvm.c:709
#9  0x00007ffff7688e40 in unroll (L=0x745b380, ud=<optimized out>) at ldo.c:442
#10 0x00007ffff76885cf in luaD_rawrunprotected (L=L@entry=0x745b380, f=f@entry=0x7ffff7688e60 <resume>, ud=0x2258060) at ldo.c:131
#11 0x00007ffff7689027 in lua_resume (L=0x745b380, from=<optimized out>, nargs=<optimized out>) at ldo.c:543
#12 0x0000000000451760 in luaT_resume (L=0x745b380, f=0x745b380, n=1) at CorsixTH/CorsixTH/Src/th_lua.h:114
#13 0x0000000000451103 in l_mainloop (L=0x700db0) at CorsixTH/CorsixTH/Src/sdl_core.cpp:260
#14 0x00007ffff7688c4d in luaD_precall (L=L@entry=0x700db0, func=<optimized out>, func@entry=0x7160f50, nresults=nresults@entry=2) at ldo.c:319
#15 0x00007ffff769483d in luaV_execute (L=L@entry=0x700db0) at lvm.c:709
#16 0x00007ffff7688f8e in luaD_call (L=L@entry=0x700db0, func=<optimized out>, nResults=nResults@entry=-1, allowyield=allowyield@entry=0) at ldo.c:402
#17 0x00007ffff768502b in lua_callk (L=0x700db0, nargs=<optimized out>, nresults=-1, ctx=<optimized out>, k=<optimized out>) at lapi.c:905
#18 0x0000000000452ac4 in CorsixTH_lua_main (L=0x700db0) at CorsixTH/CorsixTH/Src/main.cpp:179

The segfault occurs with this version 3.0~rc1+git+321c0c9-1 as made available here: https://packages.debian.org/sid/lua-socket

Can you narrow the problem down to something smaller?

How would you suggest doing that?

By creating a .lua file with no dependencies that I can run and that crashes. I don't know what CorsixTH is or how it uses Lua or LuaSocket.

@grote would you be able to see if you can reproduce this, copying just the code from App:checkForUpdates relevant to luasocket and running it with lua on your system?
Something like:
  local update_url = 'http://www.corsixth.com/check-for-updates'

  local success, socket = pcall(require, "socket")
  local http = require "socket.http"
  local url = require "socket.url"

  local update_body, status, headers = http.request(update_url)

I don't know nothing about lua, but if I put this code into a file and run it with lua test.lua I get no output.

Ok, I don't know then. I guess I'll have to try out a debian vm and see if I can reproduce; and if I can, whether I can reproduce with debug builds so we can tell what's going on. CorsixTH doesn't do anything fancy with luasocket, it just downloads the table at that url, sanity checks it, and displays the result if necessary.

Don't you have to print something to see something? url.parse doesn't  print anything. Try
local update_url = 'http://www.corsixth.com/check-for-updates'
local success, socket = pcall(require, "socket")
local http = require "socket.http"
local url = require "socket.url"
local update_body, status, headers = http.request(update_url)
print(update_body)
print(status)
for i,v in pairs(headers) do print(i,v) end

No problem here:
return {
  major = 0,
  minor = 6,
  revision = 0,
  changelog_en = 'The latest version of CorsixTH features - among other things - an integrated Map Editor, custom campaigns, drug price impact, and as usual a lot of bug fixes.',
  download_url = 'http://corsixth.com/downloads.html',
}

200
etag    "a42322-11f-8255d80"
content-type    text/plain; charset=UTF-8
content-length  287
last-modified   Wed, 01 Jun 2016 15:20:38 GMT
connection      close
date    Sat, 18 Jun 2016 16:04:01 GMT
set-cookie      __cfduid=d03bf3bb0b4374095f7fbabed2d336c941466265840; expires=Sun, 18-Jun-17 16:04:00 GMT; path=/; domain=.corsixth.com; HttpOnly
server  cloudflare-nginx
accept-ranges   bytes
cf-ray  2b4ff98143484c24-GRU```

I have been able to reproduce this in a Debian sid VM, with lua-socket version 3.0~rc1+git+321c0c9-1 (created 2015-08-14) which appears to have been pulled from https://github.com/diegonehab/luasocket/commit/321c0c9b1f7b6b83cd83b58e7e259f53eca69373 (created 2015-03-03). Reverting to version 3.0~rc1-3 (created 2013-08-12) pulled from https://github.com/diegonehab/luasocket/releases/tag/v3.0-rc1 (created 2013-06-04) replaces the segfault with this error message on update check:
---------------------------------------------------------------

Welcome to CorsixTH v0.60!

---------------------------------------------------------------

This window will display useful information if an error occurs.

---------------------------------------------------------------

Checking for CorsixTH updates...
An error has occurred!
Almost anything can be the cause, but the detailed information below can help the developers find the source of the error.
Running: The movie_over handler.
A stack trace is included below, and the handler has been disconnected.
/usr/share/lua/5.2/socket/http.lua:189: use of undeclared variable 'PROXY'
stack traceback:
        [C]: in function 'request'
        ./Lua/app.lua:1448: in function 'checkForUpdates'
        ./Lua/app.lua:463: in function 'loadMainMenu'
        ./Lua/app.lua:293: in function 'callback_on_destroy_movie'
        ./Lua/movie_player.lua:260: in function '_destroyMovie'
        ./Lua/movie_player.lua:229: in function 'onMovieOver'
        ./Lua/app.lua:1078: in function <./Lua/app.lua:1077>
        (...tail calls...)
        ./Lua/app.lua:901: in function <./Lua/app.lua:896>

Again, I don't know anything about lua, but I can run any tests you like.

The PROXY error is known and happens on all platforms. Could you try with latest master instead.

Ok, so I tried with latest master, got segfault on CorsixTH's update check, then I bisected:

30a64c5 (master) bad
321c0c9 (debian sid) bad
22cd583 (v3.0-rc1, debian jessie) good
583257c bad
396e9e5 bad
87d72dc bad
1f9ccb2 bad (first bad commit)
ddf88ac good

1f9ccb2b586c3a7e29db3c99a23ac1cee6907cf2 is the first bad commit
commit 1f9ccb2b586c3a7e29db3c99a23ac1cee6907cf2
Author: Pierre Chapuis<EMAIL_ADDRESS>Date:   Fri Jul 5 18:00:29 2013 +0200

    http: look for PROXY in _M, not as a global

:040000 040000 6f8b5a2fb0660ef15703297b8dee22eddd54d178 8331b987f3702d4ce0053625e4b5a94549df62c7 M	src

I know it's an ancient version, but hope that helps.

What was the conclusion here?

I'm not sure, that's a strange bisect - we weren't looking for the PROXY error (which is already fixed here), we were looking for the segfault error.

This seems to be caused by a conflict between the buffer_init function of libtwolame and the one provided by luasocket.
Putting a breakpoint on buffer_init I see the following backtrace:
#0  0x00007fffed3e0420 in buffer_init ()
   from /usr/lib/x86_64-linux-gnu/libtwolame.so.0
#1  0x00007fffd40ccfb5 in ?? ()
   from /usr/lib/x86_64-linux-gnu/lua/5.2/socket/core.so
#2  0x00007ffff7687c75 in ?? () from /usr/lib/x86_64-linux-gnu/liblua5.2.so.0

Here you can see luasocket calling into buffer_init from libtwolame. A simple hack to src/buffer.h makes the problem go away:
--- luasocket-3.0~rc1+git+321c0c9.orig/src/buffer.h
+++ luasocket-3.0~rc1+git+321c0c9/src/buffer.h
@@ -34,6 +34,8 @@ typedef struct t_buffer_ {
 } t_buffer;
 typedef t_buffer *p_buffer;

+#define buffer_init foo_bar_buffer_init
+
 int buffer_open(lua_State *L);
 void buffer_init(p_buffer buf, p_io io, p_timeout tm);
 int buffer_meth_send(lua_State *L, p_buffer buf);

The root cause looks like a patch that Debian has added. Dropping that patch also seems to have fixed the problem.

This is odd. If the library has been built correctly, the reference to buffer_init should have been resolved internally, no? Or is that function marked for export because some other dependency (like LuaSec) needs it? Either way, I could try to prefix every symbol with ls_. This would break LuaSec, though.

I'm not familiar with how LuaSec integrates with luasocket. Debian have a patch they apply which modifies various functions (including buffer_init) to have different linkage. This would allow e.g. LuaSec to use (or override) the implementation of those functions. However it looks like LuaSec contains it's own implementation of those functions so I've no idea why the patch is needed (or indeed how this works on distributions other than Debian which don't seem to include a similar patch). Prefixing the function names would reduce the possibility of conflicts but would obviously cause compatibility problems with any other code using those names, but if there is a good reason for the linkage change that would seem to be the best option.
I've commented on the Debian bug (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=822825) - hopefully the Debian maintainer will be able to shed some light on the purpose of the patch currently applied.

Nothing there yet, eh? :(

Any news, new information, or anything to be done here?
We're reviewing open items in preparation for a release.  If action is needed here, please add a comment.  Otherwise, this issue will be closed on or after 24-Feb-2019.
Thanks!

I haven't heard any complaints about this in years, but I also don't use Debian and the CorsixTH dpkg disables luasocket.
As far as I'm concerned this isn't your bug anyway.

I think I stepped on this bug again today :disappointed:
When using lighttpd (a common webserver) with mod_magnet (basically lua CGI) this can be reproduced very easily.

This is the only code needed to trigger the bug:
http = require("socket.http")

r, c = http.request {
	url = "https://tbspace.de",
}

lighty.content = { r }
return 200

I'll go ask in the Debian Bugtracker for more info. It still happens in Debian sid at 10 Feb 2020 and in the current stable release (currently buster).

The solution to the problem remains the same. I don't mind that people modify LuaSocket to export internal symbols I never intended to be exported. However, if you are going to do that, please make sure the LuaSocket implementation uses the internal symbols it exported, rather than importing them from somewhere else, either by prefixing the symbols with something unique or, say, by using -Bsymbolic when linking LuaSocket.


Move hermes confs to zookeeper

Move hemres configs to zookeeper (AppController writes configs to zookeeper, Hermes watches zookeeper nodes and applies changes without restart).
Handle error when collecting cluster stats and report failures.

It also includes changes from https://github.com/AppScale/appscale/pull/2466:

Profiling interval become configurable in runtime.
Hermes writes fewer logs.
Fixed bug with getting UTC timestamp for profile log.

https://ocd.appscale.com:8080/job/Daily Build/3504/

Run failed tests again, all blue:
https://ocd.appscale.com:8080/job/Hawkeye-Advanced-8/512/
https://ocd.appscale.com:8080/job/Hawkeye-Simple-8/562/
https://ocd.appscale.com:8080/job/Hawkeye/11401/

https://ocd.appscale.com:8080/job/Daily Build/3586/
Still blue after resolving merge conflicts.

https://ocd.appscale.com:8080/job/Daily Build/3600/
All blue

https://ocd.appscale.com:8080/job/Daily Build/3678/ - in progress

https://ocd.appscale.com:8080/job/Daily Build/3689/
This one looks better


Shorten the gui.js bookmarklet

This just annoyed me.

Note: I didnt test this yet. As far as I know, this works, but you should test it just in case.

You can't just put something here without testing it

PS: If you're trying to show off your master coding skills (or try to say that you're a dev for this), this is the SIMPLEST code change I've seen so far. Plus, any online tool can do this.

Good grief, I'm on my phone.

this is the SIMPLEST code change
It is. It's the low-hanging fruit.

I'll test this really quickly when I can. @yeeteeyt

Someone please test if this does work.

Try now @yeeteeyt @3kh0
It works for me.

I can't test it RN because I do not have my school Chromebook, can you open another pull with this since it is being funny?

No, this should work

I had to change the URL back to the original longer URL

does not let me

Try now. I force-pushed it back.

Yo @3kh0  can I have permission to merge requests

Hold on, I'll force-push it back...
-------- Original Message --------
On Oct 31, 2022, 12:15, Echo < @.> wrote:
￼
does not let me
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.￼Message ID: @.>
[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "[https://github.com](https://github.com/3kh0/ext-remover/pull/185#issuecomment-1297552346", "url": "[https://github.com](https://github.com/3kh0/ext-remover/pull/185#issuecomment-1297552346", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

k

Sorry, I tried to send that as an email yesterday and it decided to send today.
-------- Original Message --------
On Nov 1, 2022, 07:30, yeetee < @.> wrote:
k
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.￼Message ID: @.>
[ { @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "[https://github.com](https://github.com/3kh0/ext-remover/pull/185#issuecomment-1298595580", "url": "[https://github.com](https://github.com/3kh0/ext-remover/pull/185#issuecomment-1298595580", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

ok

yeeteeyt like kids

🤓

@JoeSwanson70 Hello, joe swanson shitpost throwaway account. Would you like to get banned like your friend?

Aren't you a little late? You can lock the PR for contributors only.

@CountBleck I know I can do that, but I'd rather get him banned from github

He is a throwaway account. He's not likely to respond. Get your priorities in order. I don't need unnecessary email notifications for nonsense like this.
-------- Original Message --------
On Nov 7, 2022, 07:40, yeetee < @.> wrote:
@CountBleck I know I can do that, but I'd rather get him banned from github
—
Reply to this email directly, view it on GitHub, or unsubscribe.
   What language was the flight computer coded in? How tall is the JTS rocket?

at least 2 kilomiles

im ceo of Ethereal areospace, so i dont speak to poor people like you yeet

Where is the office? What language was the flight computer coded in? How tall is the JTS rocket?

bangalore, india

What? No lmao

according to my caulculations

you repeated the same thing twice

are you familiar with a thing called lag? I'm at the office right now, and as an office should, it has many people... that use wifi

then do your work instead of being on GitHub

my god you're so low that you changed your pfp to try to mock me

you repeated the same thing twice

are you familiar with a thing called lag? I'm at the office right now, and as an office should, it has many people... that use wifi

then do your work instead of being on GitHub

I'm the ceo you halfwit

my god you're so low that you changed your pfp to try to mock me
ur the ceo, and ur making school exploits

yeah do your job bozo

my god you're so low that you changed your pfp to try to mock me
ur the ceo, and ur making school exploits

what?

yeah do your job bozo

in one ear, out the other...

im ceo of etherium air space

it would be one thing if you were a just some employee but being ceo people are counting on you to do your job

sure

the rocket will have two gigahertz of thrust making it go 40 mpg

it would be one thing if you were a just some employee but being ceo people are counting on you to do your job

what do you think ceo's do?


Blot-Machine

Write a short description about your art piece, such as what your artwork represents and your experience drawing it!
Next, make sure you go through each item in the following checklist. Delete this line after!

[ ✅] I have read the steps to getting a blot
[ ✅] I have read all the submission requirements and follow them
[ ✅] I am submitting art that...

[ ✅] is algorithmically generated (will meaningfully change each time the program is run)
[ ✅] is your own work that is not copied from somewhere else
[ ✅] is drawable through the editor(does not use outside tools)
[ ✅] doesn't call Math.random() (See the documentation on randomness)
[ ✅] is drawable on Blot itself (doesn't have lines overlap more than 5 times)

[ ✅] Doesn't overlap lines more than 5 times
[ ✅] Fits in the working area of 125x125mm
[ ✅] Doesn't rely on colour to look good

If you used based your art on something else, used a tutorial, or remixed it from something else, please link it here:
Lastly, link a picture of your art piece here:

Hey! I will be closing this PR. Please note you will receive further information as to why on Slack.


Segfault when case matching on lambda-encoding containing primitives

package Test where

def State: Type =
  @self ∀
  (0 P: ∀ State -> Type)
  (& new: ∀ (pos: #Nat) (txt: #Text)
    -> P (data λ P n => n pos txt)
  )
  -> P self

def new
  (pos: #Nat)
  (txt: #Text)
  : State
  = data λ P n => n pos txt

def ste: State = new 0 "foo"

def main: ∀
  (0 P: ∀ State -> Type)
  (& new: ∀ (pos: #Nat) (txt: #Text)
    -> P (data λ P n => n pos txt)
  )
  -> P ste
  = (case ste)

Checking and running:
λ yatima check Test.ya
Checking package Test at bafy2bzacecyxwnicyh5d2c5f5az4m66472o2i2wvlcrqfzfbhhiscuozofpra
Checking definitions:
✓ State: Type
✓ new: ∀ (pos: #Nat) (txt: #Text) -> State
✓ ste: State
✓ main: ∀ (0 P: ∀ (_: State) -> Type) (& new: ∀ (pos: #Nat) (txt: #Text) -> P (data λ P n => n pos txt)) -> P ste
~/introit jcb/parser*
λ yatima run Test.ya
fish: Job 1, 'yatima run Test.ya' terminated by signal SIGSEGV (Address boundary error)

Issue fixed at gb/case-fix


sched/tasklist: replace task status list with macro definition

Summary
sched/tasklist: replace task status list with macro definition
replace to macro will help to extend the scheduling implementation
Signed-off-by: chao an<EMAIL_ADDRESS>Impact
NOTE: This change will not affect code size or performance
N/A
Testing
ci-check

@anchao Does replacing global variables with macros make the code safer or is it just aesthetic?

The relevant changes to global variables are to prepare to support AMP mode. since some architectures with high real-time requirements doesn't support the cache coherency across cores within the same cluster(Cortex-R52/Tricore), such architectures could only run in AMP mode.
|   APP 0  |  APP 1   |  APP 2   |  APP 3   |  <- Application bound to CPU
---------------------------------------------
|  Data[0] |  Data[1] |  Data[2] |  Data[3] |  <- NuttX Kernel Data supports multiple CPU instances
---------------------------------------------
|                Share Code                 |  <- NuttX kernel code shared for all CPUs
---------------------------------------------
|   UART 0 |   SPI 0  |   SPI 1  |   I2C 0  |  <- Driver is only registered to CPUs with application needs
---------------------------------------------
|  TIME 0  |  TIME 1  |  TIME 2  |  TIME 3  |  <- Core/CPU timers
---------------------------------------------
|   CPU0   |   CPU1   |   CPU2   |   CPU3   |  <- CPUs run independently

Some subsystem data does not need to be duplicated, especially the components bound to the application.
For shared hardware devices, Use spinlock to avoid race-condition for multi-core.
|   APP 0  |  APP 1   |  APP 2   |  APP 3   |
---------------------------------------------
| NetStack |  BTStack |  AUDIO   |   ...    |  <- Components bound to the application, data no need to duplicate.
---------------------------------------------
|                Share Code                 |
---------------------------------------------
|      Share UART (Protected by Spinlock)   |  <- Driver shared for all CPUS will protected by spinlock(e.g print logs)
---------------------------------------------
|   CPU0   |   CPU1   |   CPU2   |   CPU3   |


Search for nested data

Duckdb does support indexing nested data so we're all good on that side already.
Therefore I simply improved the indexable column detection to check for nested data.
The old code used to index all the columns as long as there is at least one non-nested string column though, so we just need to refresh the datasets with the "no indexable columns" error.
Close https://github.com/huggingface/datasets-server/issues/1710

Codecov Report
Patch coverage: 100.00% and project coverage change: +1.97% :tada:

Comparison is base (008c0c8) 90.43% compared to head (0a99bf2) 92.40%.
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1735      +/-   ##
==========================================
+ Coverage   90.43%   92.40%   +1.97%
==========================================
  Files         211       83     -128
  Lines       12869     5810    -7059
==========================================
- Hits        11638     5369    -6269
+ Misses       1231      441     -790

Flag
Coverage Δ

jobs_cache_maintenance
?

jobs_mongodb_migration
?

libs_libcommon
?

services_admin
?

services_api
?

services_rows
?

services_search
?

services_worker
92.40% <100.00%> (+0.04%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed
Coverage Δ

...orker/src/worker/job_runners/split/duckdb_index.py
88.81% <100.00%> (+1.11%)
:arrow_up:

...orker/tests/job_runners/split/test_duckdb_index.py
100.00% <100.00%> (ø)

... and 137 files with indirect coverage changes

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

oops forgot to mark as "ready for review"

For now, LDJnr/Puffin cannot be searched into. I'll refresh it.

It now works! Congrats @lhoestq


Beast and std::bind vs boost::bind

Hi there!
I am quite curious should I use std::bind() or boost::bind() with Boost/Beast code.
Examples from pure Boost currently using boost::bind(). But in some cases (like async_accept() from boost::asio::ip::tcp::acceptor) I can't use boost::bind() on pure boost.
At the same time Beast examples using std::bind().
Is there any considerations/recommendations regarding this matter?

You can use either one, as long as you don't try to mix the placeholders. If you use boost::bind, you have to use boost placeholders. If you use std::bind, you have to use std placeholders.
However, Beast 1.70 introduces a new function bind_front_handler which is more lightweight, and doesn't use placeholders at all, so it is recommended to use that instead of bind. All the examples have been updated.

What about boost::beast::bind_handler:

The passed handler and arguments are forwarded into the returned handler, whose associated allocator and associated executor will will be the same as those of the original handler.

?

bind_handler suffers from the same problem as boost::bind and std::bind. It supports placeholders, and is heavy to compile. I try not to use it anymore, because bind_front_handler is faster and lighter on builds.

Thank you Vinnie!
Especially for insight into bind alternatives.

@vinniefalco How does bind_front_handler compare to a lambda, in terms of being lightweight?

no idea


Air connect stop automatically after 1s

Describe the bug
Airconnect stop automatically after running 1s.
To Reproduce
Steps to reproduce the behavior.
Just start airconnect from DSM dashboard.
Expected behavior
A clear and concise description of what you expected to happen.
Logfiles
If applicable, add logfiles to help understand your problem.
airconnect.log
Hardware
Device on which you are running AirConnect-Synology):

Synology NAS or Router model name: [e.g. ds415+]
DS220+
(optional) DSM Version [e.g. DSM 6.2.3-25426 Update 3]
DSM 7.2.1-69057 Update 4

Package
Which package from the releases page have you downloaded):

package name: [e.g. AirConnect-x86-64-<IP_ADDRESS>-20201229.spk or the url]

AirConnect-dsm7-x86_64-static-1.8.3-20240327.spk
Additional context
Add any other context about the problem here.

Could you please try the non static version?
x84_64 without static

Sorry, same result.

Ok, thanks, will check it tomorrow. Changed 1.8.3 back to pre-release.

Same problem here with both packages.
Nothing very meaningful in the logs, I'm afraid:
[17:17:21] Starting AirConnect ... [17:17:21] Linking airconnect configuration to "/volume1/airconnect/airconnect.conf". [17:17:21] Linking airupnp configuration to "/volume1/airconnect/config.xml". [17:17:21] Starting airupnp on <IP_ADDRESS>:49154 [17:17:21] AirConnect successfully started on ip "<IP_ADDRESS>"! [17:17:22] Status: airupnp not running! [17:17:22] Status: airupnp not running! [17:17:35] Status: airupnp not running! [17:17:50] Status: airupnp not running!
Rolled back to  1.7.0 and it's working fine again

@WiteWulf try 1.8.1, works faster than 1.7.0 on my setup:
https://github.com/eizedev/AirConnect-Synology/files/14075677/AirConnect-Synology-1.8.1.zip

The Easter holidays were a little more complicated than expected. I'm still looking at it. Please stick with 1.7.0 or the zipped 1.8.1 for testing. Thank you

Should now be fixed in the latest release: https://github.com/eizedev/AirConnect-Synology/releases/tag/1.8.3-20240403
thanks to @seiryIsNotABlock @seiry for your help to find the problem and creating the pull request!!


Game freezes if inputting too fast

(When using Q in Windows CMD)

So far it seems to only occur when running Q in command prompt, even Git Bash for Wndows is running okay so closing for now


tensorflow-addons-0.14.0 or higher?

Dear Reader,
I have tensorflow version 2.6 running on a M1 mac, and have been using it through R + Keras API in an analysis. I would also like to use the tensorflow-addons package, however I don't think I can use the given version, i.e. 0.10.0, in my analysis? I use wonder if there is a way to use the tensorflow-addons-0.10.0 package in my current setup, or do I have to down grade my tensorflow version to 2.2?
Kindest Regards,
Colin.

Cool, so it works with the setup I have. Running the example on the README page I found (with my setup) an issue using optimizer_radam()...
""
Error in py_call_impl(callable, dots$args, dots$keywords) :
TypeError: type of argument "total_steps" must be int; got float instead
""
... I'm not passing the parameter "total_steps" a value, so might be internal? But I got the full example working by just using optimizer_adam()
Cheers,
Colin.

Hi could you pass this argument manually and set it as integer, please?
E.g. total_steps = as.integer(0)


Stop control buttons from 'remain selected' after being clicked

Problem
When you click zoom in/zoom out the button you just clicked remains a darkened colour, until you click away. I would like the buttons to only change colour while you are hovering over them and then go back to normal once you are no longer hovering over them.
Google maps doesn't do this, and from a user interface perspective it doesn't make sense.
Solution
This is all done in ol.css from lines 162 to 166
I want to get rid of the line '.ol-control button:focus' from the below code so that the same behavior that is applied to hover does not get applied to the focus.
.ol-control button:hover,
.ol-control button:focus {
    text-decoration: none;
    background-color: rgba(0,60,136,0.7);
}

Having visual feedback on focused button sounds like a good idea, for accessibility purpose. It's easy to override if you don't like it, but should be kept as a default in the library imho.

But when you click zoom in and then move away it shouldn't remain a darker colour. That doesn't make sense, and it's not how other maps do it for that same reason.
Google, leaflet, Bing and Apple all have the behavior I'm describing

Bing buttons are triggerable with the keyboard (at least they're coherent in bad practices), & google maps is, in a very opinionated way:

Having button focused after click is the default browser behaviour, I still think it's a bad idea to avoid it, moreover when the behavior you're expecting is achievable with 2 lines of css.
But I'm open to hear other's opinion about it.

I agree with @frasersab that control buttons should not remain displayed focused after moving the pointer away from them. The default should be to not focus on a standard map, and only focus when the map container has a tabindex attribute, which is needed to enable accessibility options anyway.

I agree with @frasersab that control buttons should not remain displayed focused after moving the pointer away from them.

It's the case if you don't focus it by clicking. If you click it, it's standard behavior to be focused.

The default should be to not focus on a standard map, and only focus when the map container has a tabindex attribute, which is needed to enable accessibility options anyway.

Well, that's not true. On the simple example, a click on the zoom button make it have focus, meaning they are keyboard triggerable after this. If you force unfocus, you need to re-focus the button before being able to trigger.
If the focus is kept, hiding it is a bad practice.


Corrupted display UI Frame with drawProgressBar

Hi there,
I think I discovered a bug with drawProgressBar when using UI to scroll frames containing a bargraph. The problem occurs only during scroll, once frame is fixed all is fine!
When scrolling coming frame containing bargraph there are some line draw on display during scroll, I circled them in red on screenshot below

Did some of you noticed this ?
here the frame code just in case

void drawFrameRF(OLEDDisplay *display, OLEDDisplayUiState* state, int16_t x, int16_t y) {
  char buff[32];
  int16_t percent;
  volatile uint8_t * p;
  int16_t yidx = 0;
  byte n;

  n = data.size;
  p = data.buffer;

  display->clear();
  display->setFont(Roboto_Condensed_Bold_Bold_16);

  if (!got_first) {
    display->setTextAlignment(TEXT_ALIGN_CENTER);
    display->drawString(x + 64, 14, "No radio data");
    display->drawString(x + 64, 34, "received yet");
  } else {
    display->setTextAlignment(TEXT_ALIGN_LEFT);
    if (data.type == RF_MOD_RFM69) {
      sprintf_P(buff, PSTR("NODE %d"), data.nodeid );
      display->drawString(x + 0, 0, buff);

      // rssi for RFM69B is -115 (0%) to 0 (100%)
      percent = (int16_t) data.rssi;
      // rssi range now 0 (0%) to 115 (100%)
      percent += 115;

      // now calc percentage
      percent = (percent * 100) / 115;

      if (percent > 100) percent = 100;
      if (percent < 0  ) percent = 0;

      // display bargraph on lcd
      display->drawProgressBar( x + 60, 4, 64 , 12, 50 /*percent*/);

      display->setFont(Roboto_Condensed_12);
      display->setTextAlignment(TEXT_ALIGN_CENTER);
      display->drawString(x + 64, 48, timeAgo(g_second-packet_last_seen));
    }
  }

  ui.disableIndicator();
}

This will be fixed by #63 in 3.2.3


Getting location permissions, any of them

Basic Information
Device type: emulator
OS version: 30
EasyPermissions version: 3.0.0
Describe the problem
I have an app that needs location, doesn't matter which permission. Since we have 2 of these, it's quite tricky, and didn't even start considering the background one.... The app works with coarse or fine, but has a better experience with fine.
If I pass this to requestPermissions with a rationale
private static final String[] LOCATION_PERMISSIONS = {
	Manifest.permission.ACCESS_FINE_LOCATION,
	Manifest.permission.ACCESS_COARSE_LOCATION
};

I get 2 permission dialogs (fine/coarse selector, and upgrade from coarse to fine) and the rationale dialog, if the user selects the coarse location to begin with. I think this is because EasyPermissions thinks that not all permissions were granted. Is there a way to solve this? I guess I need or instead of and relationship between items in the list?

You can implement EasyPermissions.PermissionCallbacks and then accept either location permission in onPermissionGranted() callback.
   @Override     public void onPermissionsGranted(int requestCode, @NonNull List<String> perms) {         if (perms.contains(Manifest.permission.ACCESS_COARSE_LOCATION) ||                 perms.contains(Manifest.permission.ACCESS_FINE_LOCATION)) {             // App can access device's location         }     }


Scroll Wheel stopped working

Microsoft PowerToys version
0.70.2
Installation method
PowerToys auto-update
Running as admin
Yes
Area(s) with issue?
Mouse Utilities
Steps to reproduce
Updated to v0.70.2 everything seemed to go OK
After I restarted the PC I could not scroll any page with the mouse scroll wheel, in any web browser, file explorer or in fact any app/program.
I found if I opened powertoys and selected the mouse utilities page I could scroll any of the background pages, minimize powertoys again and it stopped working!! No change to any setting before or after the update.
It must be a new BUG!!
Rolled back PC to before update everything is OK again..........
What a pain!!!
✔ Expected Behavior
See Above
❌ Actual Behavior
See Above
Other Software
Windows 11 Pro and all other software

Hi, thanks for reaching out. Could you attach /bugreport?

Sorry I can't help you with this.
When I found the problem I restored my PC with a system image from yesterday before the power toys update and switched off power toys update. I'm now back on v0.70.0 everything behaves normally now.
Regards Bob
On 14 Dec 2023, 17:19, at 17:19, "microsoft-github-policy-service[bot]" @.***> wrote:

Hi there!We need a bit more information to really debug this
issue. Can you add a "Report Bug" zip file here?    Then drag the zipfile
from your desktop onto the GitHub comment box in this issue.  Thanks!

--
Reply to this email directly or view it on GitHub:
https://github.com/microsoft/PowerToys/issues/30447#issuecomment-1856259311
You are receiving this because you authored the thread.
Message ID: @.***>

Since you are using a version 6 behind, I will close this issue. If you install v0.76.2 and the problem exists please reopen.

Sorry I must have made a typo
I am using v0.76.0 My problem is with ).76.2
Regards
Bob
------ Original Message ------
From "Joseph Finney" @.>
To "microsoft/PowerToys" @.>
Cc "BobbyBare" @.>; "Author"
@.>
Date 17/12/2023 01:50:14
Subject Re: [microsoft/PowerToys] Scroll Wheel stopped working (Issue
#30447)

Since you are using a version 6 behind, I will close this issue. If you
install v0.76.2 and the problem exists please reopen.
—
Reply to this email directly, view it on GitHub
https://github.com/microsoft/PowerToys/issues/30447#issuecomment-1859010816,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AWZE4YKD657O4ZTCV7RZ2KDYJZFVNAVCNFSM6AAAAABAU6IFL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGAYTAOBRGY.
You are receiving this because you authored the thread.Message ID:
@.***>

@BobbyBare would you please upload a /bugreport via github.com

Hi microsoft/PowerToys
Please close this ticket.
A Clean install fixed this problem, the update routine must have failed
of corrupted the update.
many thanks Bob
------ Original Message ------
From "microsoft-github-policy-service[bot]" @.>
To "microsoft/PowerToys" @.>
Cc "BobbyBare" @.>; "Mention"
@.>
Date 17/12/2023 15:57:29
Subject Re: [microsoft/PowerToys] Scroll Wheel stopped working (Issue
#30447)

Hi there!
We need a bit more information to really debug this issue. Can you add
a "Report Bug" zip file here?  Then drag the zipfile from your desktop onto
the GitHub comment box in this issue. Thanks!
Report Bug
https://user-images.githubusercontent.com/11349917/133042052-4975be21-4699-4363-83c9-a8e1869d079d.png
—
Reply to this email directly, view it on GitHub
https://github.com/microsoft/PowerToys/issues/30447#issuecomment-1859209170,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AWZE4YO4KEMG4UICQAKQ6JLYJ4I6TAVCNFSM6AAAAABAU6IFL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGIYDSMJXGA.
You are receiving this because you were mentioned.Message ID:
@.***>

Hi microsoft/PowerToys
Please close this ticket.
A Clean install fixed this problem, the update routine must have failed
of corrupted the update.
many thanks Bob
------ Original Message ------
From "Joseph Finney" @.>
To "microsoft/PowerToys" @.>
Cc "BobbyBare" @.>; "Mention"
@.>
Date 17/12/2023 15:57:19
Subject Re: [microsoft/PowerToys] Scroll Wheel stopped working (Issue
#30447)

@BobbyBare https://github.com/BobbyBare would you please upload a
/bugreport via github.com
—
Reply to this email directly, view it on GitHub
https://github.com/microsoft/PowerToys/issues/30447#issuecomment-1859209143,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AWZE4YLPREGYRIEFTVUWFR3YJ4I57AVCNFSM6AAAAABAU6IFL2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGIYDSMJUGM.
You are receiving this because you were mentioned.Message ID:
@.***>


Loading of ClearScriptV8.dll

I want to include ClearScript into my project, for scripting purposes (of course ;-))
The project itself is a DLL in a managed environment. That environment, allegedly, uses a private ManagedLoader with a custom AssemblyLoadEventHandler. This handler checks DLL-Names before loading them into the App-Domain.
It does so by splitting the assembly Name at "," with the help of "IndexOf" and "Substring". But that fails with ClearScriptV8, because of a programming mistake in that MangedLoader. (It does not check for IndexOf == -1)
I traced it fourther down to V8Proxy.cs, which loads the ClearScriptV8(-64).dll into the application.
It uses Assembly.Load("ClearScriptV8").
I cannot change the ManagedLoader, because its part of the 3rd party environment. Filing a bug would also not result in a near time solution.
Is there anything I can do on ClearScript side?
The worst I could imagine, is forking a clone of the repo and change that single line.
Is there any reason, you did not specify the Assembly-Name with that full blown identification string?
(Sorry for not using the proper vocabulary. I am rather new to .NET and it is one of my least used platforms at the moment.)

Hi @ChrisHuebsch-FLIG,

It does so by splitting the assembly Name at "," with the help of "IndexOf" and "Substring".

An assembly's Name (its so-called simple name) is extremely unlikely to have embedded commas.
Its FullName usually does, but that's true of ClearScript assemblies as well, so we're not sure how ClearScript differs in this regard.
Can you clarify?
Thanks!

The AssemblyResolveHandler, which is installed into my ApplicationContext, executes these lines, when it needs to load an additional dll:
private Assembly AssemblyResolveHandler(object sender, ResolveEventArgs args)
{
Trace.WriteLine("Resolve failed: " + args.Name);
int length = args.Name.IndexOf(',');
string str1 = args.Name.Substring(0, length);
// ...
}
V8Proxy uses a short name (or simple name). This causes the code above to run into an ArgumentOutOfRangeException, because length is -1.
According to the webpage, Assembly.Load() allows short or long names.
https://docs.microsoft.com/en-us/dotnet/api/system.reflection.assembly.load?view=netframework-4.8#System_Reflection_Assembly_Load_System_String_
But when opening the Assembly.Load() function in ObjectBrowser in Visual Studio, the comments show this:

So there are two colliding definitions of "Load()" - one in the Help library, one in the code itself.
In my environment the short name won't load because of that mistake.
Is there any method to get it loaded anyways?
Perhaps there is some method of "preloading" the V8-Library, so it won't trigger the AssemblyResolveHandler.

Hi @ChrisHuebsch-FLIG,
Ah, it's the assembly resolution event, not the assembly load event.
Ironically, Assembly.Load("ClearScriptV8") is expected to fail in most cases; ClearScript just doesn't handle the exception that's thrown in your case. The call was added long ago at the request of a user who needed a hook for loading the V8 assemblies from a custom location.
Anyway, we'll harden the call in the next ClearScript version, but we do recommend that you also report the IndexOf issue to its owners.
In the meantime, we're not aware of any clean way to work around the problem. However, you might be able to use a hack to disable the faulty event handler temporarily. Is that something you're willing to do? If so, let us know if you're using .NET Framework or .NET Core.
Thanks!

That's awesome! Thank you!

Hi @ChrisHuebsch-FLIG,
We'll assume that you're on .NET Framework, as we can't reproduce your issue on .NET Core. Apparently the latter always passes a full name to the assembly resolution handler, commas and all.
Try executing the following before instantiating your first script engine. Note that this is 100% reverse-engineered hack code that's not guaranteed to work and should be jettisoned as soon as you deploy the next version of ClearScript or a fixed version of the assembly resolution handler:
var field = typeof(AppDomain).GetField("_AssemblyResolve", BindingFlags.Instance | BindingFlags.NonPublic);
var handlers = ((Delegate)field.GetValue(AppDomain.CurrentDomain)).GetInvocationList();
Array.ForEach(handlers, handler => AppDomain.CurrentDomain.AssemblyResolve -= (ResolveEventHandler)handler);
using (new V8ScriptEngine()) { }
Array.ForEach(handlers, handler => AppDomain.CurrentDomain.AssemblyResolve += (ResolveEventHandler)handler);

Also note that this code should only be executed once, as ClearScript caches the V8 assembly and bypasses the loading process thereafter.
Good luck!

Indeed, I use .NET 4.x and not Core.
It's working. I am deeply impressed. Thank you so much.
That's the kind of support, which makes using this project a great experience.

Fixed in Version 6.0.2.

I can confirm the fix works!
Big thank you!