PR: Added support for sphinx_math_dollar in help text generation

Description of Changes
Added support for the sphinx_math_dollar plugin that e.g. SymPy uses.
https://www.sympy.org/sphinx-math-dollar/
The idea is that math equations can be marked using the LaTeX-style $...$ or $$...$$ instead of the more tedious Sphinz marking.

[ ] Wrote at least one-line docstrings (for any new functions)
[ ] Added unit test(s) covering the changes (if testable)

[x] Included a screenshot or animation (if affecting the UI, see Licecap)

With:

Without:

This checks if sphinx_math_dollar exists and if so adds it to the sphinx configuration (assuming that MathJax is used).
Not clear how to add a test for it. It seems like there are no tests for math rendering in the help plugin at all.
If it makes sense, I can add an additional check box for it in the preferences. But I think that if someone has installed sphinx_math_dollar (it is not a mandatory dependency of SymPy), they probably would like to use it by default.
Issue(s) Resolved

Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:
oscargus, Oscar Gustafsson

Finally got around to updating this. Took a bit of time as sphinx-math-dollar does not have a wheel and although the solution wasn't that tricky in the end it wasn't straightforward for me...
I'd say that the logic is a bit more correct now since earlier on packages in SKIP_PACKAGES was not correctly handled as modules (as the set subtraction was on strings with version numbers). Although it doesn't really makes any difference, it should be better now.
Also, I added a feature so that all packages with - in them are replaced with _ when the modules are asked for. This is probably the best guess one can make.
The MacOS-app failure seems unrelated and maybe I should have merged against 5.x instead?


Fix 2.8.7 download link

Windows download link for 2.8.7 is broken because the actual artefact has got version 2.8.6. Both, docs.daml.com as well as the downloadable PDF are affected.
This is the result of a "composite release" whereas some artefacts have a different version than the Daml Enterprise release version stated on GitHub, that is 2.8.7.

Additionally broken links:

https://docs.daml.com/app-dev/grpc/#get-started


[MSYS-646]Working on fix for minimum package versioning bug

In progress
Description
[Please describe what this change achieves]
Issues Resolved
[List any existing issues this PR resolves, or any Discourse or
StackOverflow discussion that's relevant]
Check List

[ ] New functionality includes tests
[ ] All tests pass
[ ] RELEASE_NOTES.md, has been updated if required (not required for bugfixes, required for API changes)
[ ] All commits have been signed-off for the Developer Certificate of Origin. See https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco

where's the bug report here?

and i'd prefer to remove the yum code entirely and replace it with the dnf package approach.  and i really do not want to implement some kind of an extension to the api here which requires continuing that forwards when the dnf package provider may have solved that.

@lamont-granquist  This issue reported on zendesk here is the link https://getchef.zendesk.com/agent/#/tickets/15964 and this is card link https://chefio.atlassian.net/browse/MSYS-646

@lamont-granquist  I tried using dnf_package to install the same package which is mentioned in the issue
I am testing this on Red Hat versiugon 7.2
I am getting error
ec2-52-91-215-65.compute-1.amazonaws.com >>>> Caused by NoMethodError: undefined method +' for false:FalseClass
ec2-52-91-215-65.compute-1.amazonaws.com /opt/chef/embedded/lib/ruby/gems/2.4.0/gems/chef-13.2.20/lib/chef/provider/package/dnf/python_helper.rb:44:in dnf_command'
When I had debug the error, came to know it tries to import dnf module at this line  but unable to do so since dnf package is not available on the system because of which it throws above error. So I have tried installing dnf on the system using yum install dnf but it says no package dnf available. So am I missing something over here while using dnf_package resource for RHEL 7.2

yeah i think per discussion i just had with @jonlives is that either him or me are going to entirely replace the yum provider with a new one based on the approach in the dnf provider, which should fix this issue.

yeah #6540 will make this go away properly in Chef-14.


Configuration RelStorage via cookiecutter

I read this and do it step by step.
I select mysql - option 2, but some steps later the the db_relstorage_postgresql_dsn is requested. Is this normal or a bug?
Select db_relstorage:
1 - postgresql
2 - mysql
3 - oracle
4 - sqlite3
Choose from 1, 2, 3, 4 [1]: 2
db_relstorage_keep_history [True]: False
db_relstorage_read_only [False]:
db_relstorage_create_schema [True]:
db_relstorage_commit_lock_timeout []:
db_relstorage_commit_lock_id []:
db_relstorage_blob_cache_size_check_external []:
db_relstorage_blob_chunk_size []:
db_relstorage_cache_local_mb []:
db_relstorage_cache_local_object_max []:
db_relstorage_cache_local_compression []:
db_relstorage_cache_local_dir []:
db_relstorage_cache_prefix []:
db_relstorage_replica_conf []:
db_relstorage_ro_replica_conf []:
db_relstorage_replica_revert_when_stale []:
db_relstorage_replica_timeout []:
Select db_relstorage_postgresql_driver:
1 - psycopg2
2 - psycopg2 gevent
3 - psycopg2cffi
4 - pg8000
Choose from 1, 2, 3, 4 [1]: 1
db_relstorage_postgresql_dsn []:

I see that all question must be anwered and the options that not needed are ignored. it's a little it too much, but ok.

Please follow https://github.com/plone/cookiecutter-zope-instance#usage to not have to answer hours of questions.


List or Uninstall by id does not return expected result

Brief description of your issue
Winget list and uninstall does not work with --id
Steps to reproduce

List packages by name:

PS C:\> winget list element
Name    Id              Version Available Source
------------------------------------------------
Element Element.Element 1.11.5            winget
Element Element.Element 1.10.1  1.11.5    winget

List or Uninstall by ID.

PS C:\> winget list --id Element.Element
No installed package found matching input criteria.
PS C:\> winget uninstall --id Element.Element
No installed package found matching input criteria.

Expected behavior
Should list packages with respective ids.
Actual behavior
No installed package found matching input criteria.
Environment
Windows Package Manager (Preview) v1.4.2161-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.22000.978
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.19.2161.0

Logs: %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir

Links
---------------------------------------------------------------------------
Privacy Statement   https://aka.ms/winget-privacy
License Agreement   https://aka.ms/winget-license
Third Party Notices https://aka.ms/winget-3rdPartyNotice
Homepage            https://aka.ms/winget
Windows Store Terms https://www.microsoft.com/en-us/storedocs/terms-of-sale

Can you provide the log files that are generated when you run winget list --id Element.Element --verbose-logs ?

Hi Trenly, I would like to help out with the logs, but they contain personal information, that I do not want to share. If there's anything specific in it, that I can share, let me know.
If it helps, I do see the same behaviour with other apps as well e.g., "Microsoft.WindowsSDK". If the issue is not specific to Element, you may find some apps on your computer as well, to reproduce.
PS C:\> winget list "Windows Software Development Kit"
Name                                                    Id                   Version          Available    Source
-----------------------------------------------------------------------------------------------------------------
Windows Software Development Kit - Windows 10.0.20348.1 Microsoft.WindowsSDK < 10.0.22000.832 10.0.22621.1 winget
Windows Software Development Kit - Windows 10.0.22621.1 Microsoft.WindowsSDK 10.0.22621.1                  winget


Unable to get the proper server member count

I am trying to publically display the member count of our server by using the ".addplaying with %users% members" command, but the Bot is not displaying the correct member count, it is off by over 300 members (around 670 instead of around 1010). What could cause the issue? Is there any way to have the Bot properly count all the members?

Not 100% certain, but perhaps that's the online/idle/dnd users?

The Bot is displaying the wrong member count with that command as well, it is the same one as displayed with the %users% command though. Discord still lists a lot more members than the Bot.

Must be the cache then. Have you done a hard restart of the bot, then started it back up? Like almost all libraries (if not all), Discord.NET caches, so perhaps that is it.

I have restarted the Bot, yes. What exactly do you mean by "hard restart"? What do I need to do?

I can confirm this happening with large servers.

Editing this line - https://github.com/Kwoth/NadekoBot/blob/1.4/src/NadekoBot/NadekoBot.cs#L75 to                 AlwaysDownloadUsers = true instead of false could fix it. I don't have the issue of member count, but if you want to try you can do that - however generally not suggested to edit source unless you know what you're doing/comfortable with it.

@shikhir-arora pretty sure its turned off so the bot isnt constantly downloading new users for large servers.
If its a small self-host it should be fine.

Yes, you don't generally want to keep that as true, but it may fix his issue. You can always revert it back after as well.

Hm, it looks different for me than in that file, and the only "AlwaysDownloadUsers" I could find was already set to true instead of false, I have attached a screenshot of what I mean.

Nevermind I figured out why it looked so weird for me, I didn't update yet. I updated and tried to change the "AlwaysDownloadUsers" to true, but it didn't fix my issue, the count is still wrong.

I also have this problem that it's showing the wrong member count. It was fine just like 5 days ago but now when I start the bot it starts to count like 800 members then 5000 members then 10890 members and stays there when my server in total has 14000 members which Dyno bot is showing correct.


ROCm Sparse Marlin Kernels

[WIP] Built on top pf https://github.com/pytorch/ao/pull/1201. This pull request introduces support for ROCm (Radeon Open Compute) for sparse marling kernel in addition to CUDA, enabling the code to run on AMD GPUs.
The main changes involve conditional compilation to handle differences between CUDA and ROCm, as well as adding ROCm-specific intrinsics for MI300x.
co-author : @lcskrishna

Key changes include:
ROCm Support in setup.py:

hip kernels generation

Conditional Compilation in CUDA Source Files:

Added conditional compilation directives to exclude certain code for ROCm and include ROCm-specific implementations.

ROCm-specific Implementations:

Implemented ROCm-specific versions of functions and macros that are different from their CUDA counterparts, ensuring compatibility and performance on AMD GPUs.

Next:

[ ] validation and benchmark across workloads on MIxxx GPUs

Do you have performance numbers by any chance relative to fp16? wanna make sure the performance improvements are competitive with CUDA

Do you have performance numbers by any chance relative to fp16? wanna make sure the performance improvements are competitive with CUDA

still WIP, but would you share the benchmark you guys are using? will tray that on mi300x when the PR is ready.

Ok holler at me again whenever you need a review. Really excited to see this land

For benchmarking it is a little ad hoc the best place for this today would be to verify on: https://github.com/pytorch/ao/blob/main/torchao/_models/llama/generate.py


Seafile in NPM

Hello, I try to get Seafile server works on my no-ip domain with NPM. It works perfect with my local address.
I have my Nextcloud and Emby installed with Dietpi-software and now I install Seafile server with docker-compose on my dietpiOS (on HP Intel miniPC).
My docker-compose looks that:
version: '2.0'
services:
  db:
    image: mariadb
    container_name: seafile-mysql
    restart: unless-stopped
    environment:
      - MYSQL_ROOT_PASSWORD=.......
      - MYSQL_LOG_CONSOLE=true
    volumes:
      - ./data/mariadb:/var/lib/mysql
    networks:
      - seafile-net

  memcached:
    image: memcached
    container_name: seafile-memcached
    restart: unless-stopped
    entrypoint: memcached -m 256
    networks:
      - seafile-net

  seafile:
    image: seafileltd/seafile-mc
    container_name: seafile
    restart: unless-stopped
    ports:
      - "8090:80"
    volumes:
      - ./data/app:/shared
    environment:
      - DB_HOST=db
      - DB_ROOT_PASSWD=.......
      - TIME_ZONE=Etc/UTC
      -<EMAIL_ADDRESS>      - SEAFILE_ADMIN_PASSWORD=..........
      - SEAFILE_SERVER_LETSENCRYPT=false
      - SEAFILE_SERVER_HOSTNAME=..........ddns.net/seafile
    depends_on:
      - db
      - memcached
    networks:
      - seafile-net

networks:
  seafile-net:

I have in NPM
Details tab:
Domain: myserver.ddns.net
Schema: https
Host IP:  <IP_ADDRESS>
Port: 80

Custom location tab:
/nextcloud
http
<IP_ADDRESS>
80

/emby
http
<IP_ADDRESS>
8096

/Seafile
http
<IP_ADDRESS>
8090

More details I give on Seafile folder but don't get help yet.
https://forum.seafile.com/t/looking-for-help-with-nginx-proxy-manager/16040
Regards.

It works perfect with my local address.

What do you mean by this exactly ?   From the host running the containers, are you able to reach Seafile at http://<IP_ADDRESS>:8090 ?

It works perfect with my local address.

What do you mean by this exactly ? From the host running the containers, are you able to reach Seafile at http://<IP_ADDRESS>:8090 ?

I mean it works with my local address https://<IP_ADDRESS>:8090 and on my externel address: https://myserver.ddns.net/seafile it looks that:
https://forum-seafile-com-backups.s3.dualstack.us-east-1.amazonaws.com/original/2X/3/3f80de35a8f7f76dcc6ff5b5e1eb4b6e4b599cb7.png
Regards.

OK, I install it with that tutorial https://itsfoss.com/deploy-seafile-server-docker/ and works fine with NPM but when I access admin settings in web panel my certificate changes as not valid and browser shows notification about danger. If I back to main window it's still wrong. I must open browser again to make Seafile work OK with SSL. :/
Regards.


Issue with k8s.io/docs/user-guide/kubectl-overview/

This is a...

[ ] Feature Request
[x] Bug Report

Problem:
The reader gets insufficient information about how patches work.
Proposed Solution:
Add a link to https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#patch-operations and/or https://github.com/kubernetes/community/blob/master/contributors/devel/strategic-merge-patch.md
Page to Update:
http://kubernetes.io/docs/user-guide/kubectl-overview/
https://kubernetes.io/docs/user-guide/kubectl/v1.7/#patch
https://kubernetes.io/docs/api-reference/v1.7/ (subsection on patching in Resource Operations)

@MikeSpreitzer :wave: Can you be more specific about what information is missing? Where did you run into difficulty?

We can link to this document:
https://kubernetes.io/docs/tasks/run-application/update-api-object-kubectl-patch/
I'll add the links.

/remove-lifecycle rotten
@steveperry-53 If it's not an issue, please close it. :)


[AppService] Fix #16681: az webapp config ssl import: fixing bug that causes failures on national clouds

Description

With the last a change, a bug was introduced that attempted to use the CertificateRegistration RP in national clouds, which causes a failure.
Testing Guide

az webapp config ssl import --resource-group rg --name app --key-vault kv --key-vault-certificate cert
History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.

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

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

[x] I adhere to the Command Guidelines.

[x] I adhere to the Error Handling Guidelines.

AppService

AppService

@qwordy @Juliehzl I am trying to write a test that runs in a national cloud, to track this scenario going forward, but it seems like the way I managed to get it working locally is not ideal, it might contain sensitive information. Is there a better way to run a test on a government cloud? thanks!

@qwordy @Juliehzl I am trying to write a test that runs in a national cloud, to track this scenario going forward, but it seems like the way I managed to get it working locally is not ideal, it might contain sensitive information. Is there a better way to run a test on a government cloud? thanks!

@qwordy @Juliehzl I am trying to write a test that runs in a national cloud, to track this scenario going forward, but it seems like the way I managed to get it working locally is not ideal, it might contain sensitive information. Is there a better way to run a test on a government cloud? thanks!

@qwordy  mainly is there a way we can run specific tests or all test on non public clouds. That way Sofi doesn't need to add a specific test for S-clouds?

@qwordy @Juliehzl I am trying to write a test that runs in a national cloud, to track this scenario going forward, but it seems like the way I managed to get it working locally is not ideal, it might contain sensitive information. Is there a better way to run a test on a government cloud? thanks!

@qwordy  mainly is there a way we can run specific tests or all test on non public clouds. That way Sofi doesn't need to add a specific test for S-clouds?

Lets get the build to pass otherwise looks good to me.

The build is failing because of the test I tried adding, I will remove it, and hopefully we can run the tests in national cloud

@singlesis Login to the cloud you want to test. Then run test lively. Replay test should be able to pass as long as live test can pass. If code in test case has sensitive information, I recommend deleting the test case. But, remember to make sure it can work in the expected cloud. If recording file has sensitive information, you can mark the test case as live_only(), then delete the recording file.

@Juliehzl could you take a look?


Need Trainning setting shared

Trying to recurrent your result in paper when applying MaskRCNN on cellpose dataset.It will be helpful if you can share your training setting of MaskRCNN. Thanks a lot.

the training settings are included in the paper, I've also uploaded the training script to the paper folder


Issue 113

This is a rebased version of https://github.com/ga4gh/beacon-team/pull/115 submitted from a branch in the main repo for collaboration. Some fixes for the build have been added on top of https://github.com/ga4gh/beacon-team/pull/115.

Whoops, wrong branch. I'll resubmit.


Improve Istio Reconciler codebase automation

Description

Istio Reconciler should have following things managed:

Function validatePath from pkg/reconciler/instances/istio/bootstrap.go should have automated tests defined.

func validatePath(path string) error

Struct TestCommanderResolver should have a mock generated by mockery. For now it does not because of circular import dependency problem.

Reasons

Istio Reconciler test automation should cover the most crucial points and follow the general convention of mocking.
Attachments

Follow-up of: https://github.com/kyma-incubator/reconciler/pull/497
See: https://github.com/kyma-incubator/reconciler/issues/270

Update:

this point is no longer applicable. The function was renamed and covered with tests in scope of this PR.
this point is still applicable.

istio reconciler will be reworded into istio manager


MPPlayer texture glitch

The texture changes randomly sometimes when both clients are at 0deg rotation.

additionally, bugs out when players FPS is low.


[WIP] Remove moi

This is the list of tasks that need to be completed before this can be merged:

[ ] Move submit_to_EBI to Qiita internal "plugin"
[ ] Move submit_to_VAMPS to Qiita internal "plugin" @josenavas
[ ] Delete create_raw_data from qiita_ware/dispatchable.py since it is not used anywhere
[ ] Move copy_raw_data to Qiita internal "plugin"
[ ] Move delete_artifact to Qiita internal "plugin"
[ ] Move create_sample_template to Qiita internal "plugin"
[ ] Move update_sample_template to Qiita internal "plugin"
[ ] Move delete_sample_template to Qiita internal "plugin"
[ ] Move update_prep_template to Qiita internal "plugin"
[ ] Move delete_sample_or_column to Qiita internal "plugin"
[ ] Remove qiita_ware/dispatchable.py
[ ] Remove qiita_ware/context.py
[ ] Remove qiita_ware/executor.py
[ ] Remove per_sample_sequences from qiita_ware/util.py since it is not used anywhere
[ ] Remove qiita_ware/wrapper.py
[ ] Update qiita_core/configuration_manager.py to remove the ipython configuration
[ ] Update the default Qiita configuration files
[ ] Update setup.py and .travis.yml
[ ] Updaet qiita_test_install to not check for ipython (is no longer a Qiita dependency)
[ ] Update installation instructions

In the list above, everything that needs to be moved to the Qiita internal plugin requires add a new command to the Qiita internal plugin and update the interface bits to look for a ProcessingJob rather than a moi job in redis

Codecov Report

Merging #2260 into dev will increase coverage by 2.8%.
The diff coverage is 98.63%.

@@            Coverage Diff            @@
##              dev    #2260     +/-   ##
=========================================
+ Coverage   92.82%   95.62%   +2.8%
=========================================
  Files         171       75     -96
  Lines       18930    13021   -5909
=========================================
- Hits        17571    12451   -5120
+ Misses       1359      570    -789

Impacted Files
Coverage Δ

qiita_db/search.py
92.56% <100%> (+0.05%)
:arrow_up:

qiita_db/util.py
96.68% <100%> (+0.12%)
:arrow_up:

qiita_db/metadata_template/util.py
90.9% <100%> (-0.07%)
:arrow_down:

qiita_db/test/test_util.py
99.61% <98.07%> (-0.18%)
:arrow_down:

qiita_pet/handlers/portal.py

qiita_pet/handlers/study_handlers/ebi_handlers.py

qiita_pet/test/test_ontology.py

...iita_pet/handlers/study_handlers/vamps_handlers.py

qiita_pet/handlers/user_handlers.py

...lers/artifact_handlers/tests/test_base_handlers.py

... and 89 more

Continue to review full report at Codecov.

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


Major code and performance improvements. Fixed SQL Injection, Baguette.

Glad to help out. :)
Fixes several bugs, security issues, performance problems and bad coding style.
Pls accept mr. x3

Interesting


Implement classifier

Tacker service needs to add a classifier resource to be able to classify traffic to a chain.

Done


init

Pull Request Template
Description
We are releasing a new Observability4rookies workshop that does not include the agent installation so we can focus more time on showcasing the power of our platform. I have not done any delete just added a totally new section under O11yrookies that excludes the installation steps.
Fixes # (issue)
Type of change
Please delete options that are not relevant.

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

How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

[ ] Test A
[ ] Test B

Checklist

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] Markdown Lint passes locally with my changes
[x] Any dependent changes have been merged and published in downstream modules

I think that if we took some of this additional content and merge it with main workshop along with the Terrafrom presetup=true we could have the perfect blend … please hold off merging until a full review … this is great additional from the first pass, thank you!

Sounds good u know what is best
On Fri, May 5, 2023 at 6:25 PM Robert Castley @.***>
wrote:

[ External sender. Exercise caution. ]
I think that if we took some of this additional content and merge it with
main workshop along with the Terrafrom presetup=true we could have the
perfect blend … please hold off merging until a full review … this is great
additional from the first pass, thank you!
—
Reply to this email directly, view it on GitHub
https://github.com/splunk/observability-workshop/pull/166#issuecomment-1536850744,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AY4PAS6G5P4PCR4E2O72LXTXEV44RANCNFSM6AAAAAAXXPS52I
.
You are receiving this because you authored the thread.Message ID:
@.***>

--
Deepti Bhutani
Senior Solutions Engineer
Splunk Inc.
Mobile:<PHONE_NUMBER>
Atlanta | AMER | APAC | EMEA

Hi Robert,
Do you know how long will it take for this to be available?
I was planning on leveraging this content for one of the internal workshop.
Thanks,
Deepti Bhutani
Senior Solutions Engineer
Splunk Inc.
Mobile:<PHONE_NUMBER>
Atlanta | AMER | APAC | EMEA
On Fri, May 5, 2023 at 6:46 PM Deepti Bhutani @.***> wrote:

Sounds good u know what is best
On Fri, May 5, 2023 at 6:25 PM Robert Castley @.***>
wrote:

[ External sender. Exercise caution. ]
I think that if we took some of this additional content and merge it with
main workshop along with the Terrafrom presetup=true we could have the
perfect blend … please hold off merging until a full review … this is great
additional from the first pass, thank you!
—
Reply to this email directly, view it on GitHub
https://github.com/splunk/observability-workshop/pull/166#issuecomment-1536850744,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AY4PAS6G5P4PCR4E2O72LXTXEV44RANCNFSM6AAAAAAXXPS52I
.
You are receiving this because you authored the thread.Message ID:
@.***>

--
Deepti Bhutani
Senior Solutions Engineer
Splunk Inc.
Mobile:<PHONE_NUMBER>
Atlanta | AMER | APAC | EMEA

This is not a breaking change. Will merge now so the workshop can go on, and we can review and consolidate when time permits.


dx cpu load is high

Problem
when I run dx serve, I see the dx process use up to 30% of all cpu.
Steps To Reproduce

create a simple test programme of dioxus,
run dx server --hot-reload --platform desktop
check the cpu load of dx process

Expected behavior
normal cpu load, like any other program like vscode, Edge, word. (< 10%)
Screenshots
If applicable, add screenshots to help explain your problem.
Environment:

Dioxus version: branch v0.5
Rust version: 1.73
OS info: Windows
App platform: desktop

Questionnaire

[x] I'm interested in fixing this myself but don't know where to start
[x] I would like to fix and I have a solution
[ ] I don't have time to fix this right now, but maybe later

I checked the code,
https://github.com/DioxusLabs/dioxus/blob/0.5/packages/cli/src/server/desktop/mod.rs#L126-L129
                let _ = local_socket_stream.set_nonblocking(true);
                move || {
                    loop {
                        match local_socket_stream.accept() {

the nonblocking and the infinite loop (without sleep inside) should be the root cause. I would suggest to use blocking mode of interprocess.

I saw you opened https://github.com/DioxusLabs/dioxus/pull/1582 which looks like it would fix this. Whenever you are done with it, you can change from a draft PR to a normal PR and I would happy to review it

#1582 is another way to fix it, which uses sleep, but now I think it's better to use blocking, what's your thought?

#1582 is another way to fix it, which uses sleep, but now I think it's better to use blocking, what's your thought?

I agree, that seems like a better solution


Question/Help: How to get and save values for users and app settings

Hi
I'm trying to use the package to store app and user specific settings, so therefor i utilize the 'extra column' feature, but... I seem to miss something.
The below only save the last value - I have autosave set to true in the config.
        setting()->setExtraColumns([ 'is_system' => 1]);
        setting()->set('default.currency', 'EUR');
        setting()->set('test', 20);

        setting()->setExtraColumns([ 'is_system' => 0]);
        setting()->set('default.currency', 'EUR');
        setting()->set('test', 10);

This saves the values correctly
        setting()->setExtraColumns([ 'is_system' => 1]);
        setting()->set('default.currency', 'EUR');
        setting()->set('test', 20);
        setting()->save();

        setting()->setExtraColumns([ 'is_system' => 0]);
        setting()->set('default.currency', 'EUR');
        setting()->set('test', 10);
        setting()->save();

in the config I have
    'fallback' => [
        'is_system' => 0
    ],

but that does not seem to have any effect on set() or get()
do I really need to call the setExtraColumns() every time I need a setting ?

Yes, you should call the setExtraColumns() every time you need settings. As for the autosave, it doesn't save when you call set method but once per runtime/request.


Misprint

Misprint

These are actually intentional to keep the log output size predictable as all of the other levels are 4 characters.


explain roughly what the new generator is doing

/assign @JoelSpeed
clear as mud.

Updated

/retest

/lgtm
/override ci/prow/e2e-upgrade-minor
We don't need e2e for a markdown only change


[Feature] Generate a all.min.js

Would be great if we have a file all.min.js with all .js there.
Congratulation for your work guys.

Agreed! Let's ask @fat.

I think @jasny started working on this here https://github.com/jasny/bootstrap/tree/2.0-uglify

It's on my list to build a service which builds the files together (this is pretty necessary for the 2.0 plugins). If anyone wants to step up and build a jsonp endpoint for this, that would be... basically amazing.

@fat, you mean something like a server side customizer as seen here http://www.initializr.com/advanced ?

I simply stick all uglified files together. Not really refined. Isn't ender suppose to be a solution for this? http://ender.no.de
P.S. Please note that I run a heavily modified fork of Bootstrap 2.0. Not all branches are intended to be merged back into the Twitter version. Each new feature is put in it's own branch before merging it into my master. So do feel free to check it out :)

The mootools crew has had one for ages which is pretty great: http://mootools.net/core/
I think @markdotto and i will want to build the interface into our docs (mark will hopefull design some hottness for it) - so we just need an api endpoint which takes some arguments and returns the built file. (probably with an option to do compressed or uncompressed code). I was thinking about using ender for it :)

I hacked a little something something as a proof of concept => http://bootstrapr.okonski.org/

Great work. Would be ever better if it have an select all button! :D

Ah yes! Done ;)

@pokonski this is awesome! i'll take a closer look in the next few days :)

@pokonski I just opened a pull request on your bootstrapr project allowing building a package pulling the latest plugins right from GitHub instead of just using the static files included.

Great! I actually considered that :)

@pokonski i ported your code to node - and implemented a simple ui in the 2.0 docs. This will likely be improved upon - but should definitely make it into 2.0 :)

Nice :-)

here's a gist: https://gist.github.com/1601997 - won't likely open source it as a project because it's so simple

@pokonski Didn't use loadbuilder/runner that was just for something at work :)

I see :)  Also that 'jsonpi' plugin is clever!

isn't it? @benvinegar is a genius and a maverick.

Thanks for the gist.


Replace deprecated body in the ES requests

Description

The get_sources function in the search_controller uses the deprecated body parameter:
https://github.com/WordPress/openverse/blob/517a16e407abad1801702b1dceabf1b0598f853b/api/api/controllers/search_controller.py#L555-L571
Deprecation warning in the CI (when running the API tests).
  /api/api/controllers/search_controller.py:682: DeprecationWarning: The 'body' parameter is deprecated and will be removed in a future version. Instead use individual parameters.
    results = settings.ES.search(index=index, body=agg_body, request_cache=True)

This can be replaced with the following code:
        aggs = {
                "unique_sources": {
                    "terms": {
                        "field": "source.keyword",
                        "size": size,
                        "order": {"_key": "desc"},
                    }
                }
        }
        try:
            results = settings.ES.search(index=index, aggs=aggs, request_cache=True)

Additional context

We recently updated ES version from 7 to 8, and started seeing the deprecation warnings in CI

Hello @obulat and @WordPress/openverse-maintainers,
I'm trying to have my first contribution on open source project and this issue look suitable for me. Can you assign me to this issue? How can I run an API tests?

Hi @ngken0995, you can find the instructions for the API tests in the docs site.

Hello @krysal,
Thank you for providing the docs site. It was super helpful. I can't join the slack channel because I'm not authorized. I'm trying to run just api/up.
rHere is the result:
env COMPOSE_PROFILES="api" just ../up env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipe dcfailed on line 141 with exit code 127 env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipedcfailed on line 141 with exit code 127 env COMPOSE_PROFILES="api" docker-compose -f docker-compose.yml up -d env: ‘docker-compose’: No such file or directory error: Recipedcfailed on line 141 with exit code 127
I'm kinda stuck trying to resolve the error. Please advise.

@ngken0995 Could try running just env before just api/up? That must complete your files.
Also, you should be able to join Slack, it's free to access for anyone, you just need a WordPress.org account first. Check this page.

Hello @krysal, I was able to run just api/up, just api/init and just api/test. I made the changes suggested above and still getting the same error when I run just api/ test. Do I need to change any test data?

@krysal, I have joined the slack channel with a WordPress.org account.
This is the result after I made the changes and ran just api/test below:
`test/test_auth.py: 7 warnings
test/test_dead_link_filter.py: 4 warnings
test/unit/controllers/test_search_controller.py: 125 warnings
/venv/lib/python3.11/site-packages/elasticsearch_dsl/search.py:712: DeprecationWarning: Passing transport options in the API method is deprecated. Use 'Elasticsearch.options()' instead.
es.search(index=self._index, body=self.to_dict(), **self._params).body,
test/test_auth.py: 14 warnings
test/test_dead_link_filter.py: 7 warnings
test/unit/controllers/test_search_controller.py: 237 warnings
test/unit/utils/test_search_context.py: 4 warnings
test/unit/views/test_media_views.py: 6 warnings
/venv/lib/python3.11/site-packages/elasticsearch_dsl/search.py:712: DeprecationWarning: The 'body' parameter is deprecated and will be removed in a future version. Instead use individual parameters.
es.search(index=self._index, body=self.to_dict(), **self._params).body,
test/unit/models/test_media_report.py::test_deindexing_creates_deleted_image_instance[image_media_type_config]
test/unit/models/test_media_report.py::test_deindexing_creates_deleted_image_instance[image_media_type_config]
test/unit/models/test_media_report.py::test_deindexing_creates_deleted_image_instance[audio_media_type_config]
test/unit/models/test_media_report.py::test_deindexing_creates_deleted_image_instance[audio_media_type_config]
/venv/lib/python3.11/site-packages/django/db/models/fields/related_lookups.py:52: RemovedInDjango50Warning: Passing unsaved model instances to related filters is deprecated.
warnings.warn(
../venv/lib/python3.11/site-packages/_pytest/cacheprovider.py:451
/venv/lib/python3.11/site-packages/_pytest/cacheprovider.py:451: PytestCacheWarning: could not create cache path /api/.pytest_cache/v/cache/nodeids: [Errno 13] Permission denied: '/api/.pytest_cache'
config.cache.set("cache/nodeids", sorted(self.cached_nodeids))
../venv/lib/python3.11/site-packages/_pytest/stepwise.py:56
/venv/lib/python3.11/site-packages/_pytest/stepwise.py:56: PytestCacheWarning: could not create cache path /api/.pytest_cache/v/cache/stepwise: [Errno 13] Permission denied: '/api/.pytest_cache'
session.config.cache.set(STEPWISE_CACHE_DIR, [])
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
Results (165.04s (0:02:45)):
559 passed
4 skipped
`

@ngken0995, It looks like all of the tests passed. There are some warnings about the deprecations, but they don't affect the 162 passed tests.

@obulat This is good to hear! I have created a PR.


Determine data type

When I called parseImageAttributes([x=800,y=640,sar=1.1,q=0.6]), I got the result  [{"x":800,"y":640,"q":0,"sar":0}]. After reading the source code, I found something wrong with data type, so I did some changes.

Is this PR converting into int a json string with this exact value: "123"?
I know that sdp-transform (the JS library) does it. But it comes with several issues such as when parsing values like a=mid. By spec they are strings, however browsers usually use a=mid:0 and a=mid:1. Those 0 and 1  must be strings rather than integers in the parsed Object.
BTW: have you run the tests? I won't accept any PR that does not pass the test units (or modifies them according to the changes in the PR if the expected results make sense): https://travis-ci.org/ibc/libsdptransform/jobs/468348603

Humm, I'm considering accepting this change. It should be carefully documented so the app does not assume that certain params are always integers, or floats or strings.

Let's do it.

This has been improved (specially the documentation) in 1.2.0


reset password functionality not available

This has been raised twice before, #220 and #247, but it looks like both are closed without it actually being fixed.  Can we leave this open until it is fixed so that it is trackable?

There are many auth methods we need to implement, and I don't think it makes sense to have each one as an issue. We plan on creating a larger issue in the future when we are ready to tackle large auth support.
In the meantime it's perfectly fine to use firebase.auth().sendPasswordResetEmail(emailAddress) from the regular SDK.

@davideast Is the pure firebase sdk included in angularfire2 and how can I access it within an angular2 .ts file? Im am only getting offered "AngularFire, FirebaseAuth, FirebaseAuthState" and the observables. Thanks!

import * as firebase from 'firebase'; should get you access. It's a bit
temperamental if you upgraded from alpha, so check some of the existing
issues if you get an error.
On Nov 18, 2016 3:02 AM, "bjornson"<EMAIL_ADDRESS>wrote:
@davideast https://github.com/davideast Is the pure firebase sdk included
in angularfire2 and how can I access it within an angular2 .ts file? Im am
only getting offered "AngularFire, FirebaseAuth, FirebaseAuthState" and the
observables. Thanks!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/angular/angularfire2/issues/374#issuecomment-261505903,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA6l5BkBl3Um9ALefl06dzUUrQwJX-TCks5q_YXPgaJpZM4JQvE-
.

@katowulf thanks for the info, I ended up injecting FirebaseApp in the contructor:
private _firebaseApp;
constructor(@Inject(FirebaseApp) firebaseApp: any){
    // listen to auth state change
    this._firebaseApp.auth().onAuthStateChanged(user => {
        if (user) {
            // is logged in
        } else {
            // is logged out
        }
    });
}

requestPasswordResetEmail(email){
    this._firebaseApp.auth().sendPasswordResetEmail(email)
                .then(response => resolve(response),
                    err => reject(err));
}

A great side effect is that the auth listener in my app now works on iOS Safari browser as well. So far the only modern browser that had any trouble with the AngularFire auth listener.

is completed now?

Where is the function to send a code to reset the password??
I can see that "confirmPasswordReset(code, newPassword)" but without a function to send the reset code, don't make sense ....


Word wrapping turns automatically on and can't be turned off

VSCode Version:1.0.0
OS Version: Windows 10

Steps to Reproduce:

Create new file
Type a string longer than 300 characters - let's say 310 times character 'a'
Create new file (using ctrl+n)
Return to the first file (using ctrl+tab)
The line you've just typed is now wrapped on viewport width. "Toggle Word Wrap" command does not have any effect until you restart the VS Code.

Maybe an extension like editorconfig?

I tried to download VSCode insider build and run it without any extensions and could still reproduce the bug.

The wrapping settings are getting an overhaul with #21262


Modules reorganization

Reorganizes the internal structure of the repo before releasing the new version

Codecov Report

Merging #140 (b73839f) into master (dfc7535) will not change coverage.
The diff coverage is 100.00%.

:exclamation: Current head b73839f differs from pull request most recent head 5adb0be. Consider uploading reports for the commit 5adb0be to get more accurate results

@@            Coverage Diff            @@
##            master      #140   +/-   ##
=========================================
  Coverage   100.00%   100.00%
=========================================
  Files           13        20    +7
  Lines          916       951   +35
=========================================
+ Hits           916       951   +35

Impacted Files
Coverage Δ

pgmax/factor/factor.py
100.00% <ø> (ø)

pgmax/factor/__init__.py
100.00% <100.00%> (ø)

pgmax/factor/enum.py
100.00% <100.00%> (ø)

pgmax/factor/logical.py
100.00% <100.00%> (ø)

pgmax/fgraph/__init__.py
100.00% <100.00%> (ø)

pgmax/fgraph/fgraph.py
100.00% <100.00%> (ø)

pgmax/fgroup/__init__.py
100.00% <100.00%> (ø)

pgmax/fgroup/enum.py
100.00% <100.00%> (ø)

pgmax/fgroup/fgroup.py
100.00% <100.00%> (ø)

pgmax/fgroup/logical.py
100.00% <100.00%> (ø)

... and 9 more

Continue to review full report at Codecov.

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


Allow all-days for getTopPageviews, Fixing #19

Thank you very much! Will draft a new release now.

Thank you very much! Will draft a new release now.

Thanks for the library, saved me some time! :D

Thanks for the library, saved me some time! :D


Add custom redis cache

For now, we do the simplest approach possible, meaning we cache the results in Redis. Using exact match strategy. In the future this should be extended to other stores and similarity based matching to enable true semantic caching. For that vector store handling needs to be added. Proper exception handling.
Validations and defaults for environment variables were added using pydantic and python-dotenv for local setup. In the future this should be used to load and validate all the env variables as it's currently the preferred go-to approach in Python world.
Tests need to be added, but for that to happen I think we should refactor the whole testing approach.
Pipelines will fail coz of some lints that are still left out there to be fixed and some tests that were broken since the beginning. We also need to work on that.
Documentation to follow if this implementation is deemed merge-worthy.

Did a small refactor and encapsualted the default cache that you've used. it probably still needs some work to achieve full integration though.

@ishaan-jaff @krrishdholakia any opinions so far guys? This needs some love & testing, will gladly help, but I need the intial approval to push things forward.

any feedback here...?

Sorry for the delay @grski will take a look and respond by EOD

thank you very much, no need to hurry if you have other priorities, the information is enough that you have this on the todo list, no worries :)

@ishaan-jaff @krrishdholakia
It's finally done. The tests pass. Linting applied. No conflicts with main (it's merged into this branch).


Improve the visibility around setting a default project.

A number of the datalab libraries require either:

Explicitly passing in a Context object with a project_id set
or
Setting the project_id in the default Context instance.

When neither of those were done, the subsequent API calls
could fail with very confusing error messages (such as "Not found").
This changes the libraries so that an error is surfaced earlier,
with an explicit explanation of what has gone wrong, and with
instructions on how to fix the issue.
Specifically, attempts to read the 'project_id' property of
a Context object will raise an error if that property has not
been set. The error message will explain that the 'project_id'
has not been set, and will suggest using the '%projects set'
cell magic to set a default project.
This change also fixes the '%projects set' cell magic so that
it actually sets the project ID to be the passed in argument,
rather than a singleton list of the literal string 'id'.

LGTM


add(plugin): tailwindcss-unsplash

Name
Link

tailwindcss-unsplash
https://github.com/shorwood/tailwindcss-unsplash

Apply unsplash.com images as background.

[x] My item is in the right category
[x] My item is logically grouped below similar items
[x] My item's name and description respects the conventions of the list
[x] My item is awesome
[x] My item is in line with the Tailwind brand usage guidelines
[x] I have read and followed the contribution guidelines

Thanks!


Package request: koha

Project description
The world's first free and open source library system
Metadata

homepage URL: https://koha-community.org/
source URL: https://git.koha-community.org/koha-community/koha
license: gpl3
platforms: linux

Add a :+1: reaction to issues you find important.

Looks like a pretty nice first package to write, so I'll give it a try.
(Want to use Koha on my NixOS'ed Raspberry Pi 4 as well, so somebody has to do it.)


Initializing root logger

I feel that the root logger should not be set by a single python package. This is creating double logging for our package that depends on Flax, Jax, etc, as we have our own handler on the logger in our namespace.
https://github.com/abseil/abseil-py/blob/3e4749af698201224165a98930c6845d8c102e0d/absl/logging/__init__.py#L559-L560
I could be wrong but in this code:
https://github.com/abseil/abseil-py/blob/3e4749af698201224165a98930c6845d8c102e0d/absl/logging/__init__.py#L1246-L1252
The handler should be added with _absl_logger.addHandler(_absl_handler) while logging.setLoggerClass(ABSLLogger) should be _absl_logger.setLoggerClass(ABSLLogger) with the surrounding lines removed

(1) Re the first reference of calling logging.basicConfig(): this is the same behavior in Python's own std logging library:
https://github.com/python/cpython/blob/8f31bf46980956c735dd18f9914f3e7144e87c77/Lib/logging/__init__.py#L2140-L2141
(2) Re the second reference, the _absl_handler is created at absl.logging import time but not added to any logger, to avoid exactly the double logging issue. The handler is only added to the root logger when use_absl_handler() is explicitly called, e.g. if you use absl.app.run() (see https://github.com/abseil/abseil-py/blob/3e4749af698201224165a98930c6845d8c102e0d/absl/app.py#L372)
So if you are seeing double logging:

because of the logging.basicConfig() call from (1), then it's the same behavior if use you std logging statements.
because of half of the logs are from the _absl_handler, then somewhere use_absl_handler() is called (or much less likely that you explicitly added _absl_handler to a logger).

Thank you, our problem was actually found elsewhere! Thanks for the info!


Get NESW of a geo-cordinate in reference to another co-ordinate

Request for a feature that can generate NESW for a given geo-cordinate based on a reference geo-cordinate

Could you please elaborate or give any references? What does NESW stands for?

Hi Esmukov,
Thanks for responding back,
NESW => North, East, South, West.
I was looking for a feature in geopy that can tell me N/E/S/W of a Targeted geo-coordinate in reference to other geo-coordinate, i.e:
Examples:
T: Target and R: Reference

New York (T) ; California (R) => East
New York (T) ; Mexico City (R) => North
Mumbai (T) ; Beijing (R) => West

I have also looked at other packages for this implementation, but there were none. Therefore I have requested for a feature.
Anyways, I have developed this functionality myself.
If you think this feature might be helpful, I'll cr8 a PR for it.

You can do that with a simple function, but you will need to add some functionality to check and handle if x0 and x1 or y0 and y1 are equal.
def cardinal_direction(p0: (float, float), p1: (float, float)):
    x0, y0 = p0
    x1, y1 = p1

    is_north = x1 > x0

    # the longitude is defined on a range from (-180, 180) if we
    # have two points p0=(0, 170) and p1=(0, -170) then y1 > y0
    # will return False although we can see on the map that p1 is east
    # of p0. Because of that we need to check if their distance shorter
    # than the maximum of the range they are defined on and if so we
    # need to invert the logic on this
    is_east = (y1 > y0) if abs(y1 - y0) < 180 else (y1 < y0)

     return ('N' if is_north else 'S') + ('E' if is_east else 'W')

This looks very efficient and neat.

@Enforcer007 ok, now I get it, thanks for the explanation.
I think this is a very specific (i.e. rarely needed) task which I don't feel is worth having in geopy. I'm open for discussion though.


Make consistent header / page container styles

It seems like each page is doing it's own approach to the header.  We should find a way to make all pages have a uniform looking h1 header.  Also, each page should have a similar container, margin, padding.

Do you prefer making a header component or just apply the same styles to every page?


Set up backend Mock Server

Set up backend server
Using ruby language
set up bash scripts and user uuid

Set up backend server
install ruby
install ruby dependencies, "bundle install"
edited the /update endpoint
edited the domain_name to $home[:domain_name]


Fix param read loop for python 3

This commit changes the 'blank parameter ID' value from empty string to empty bytes, so that it can be packed for transmission to the copter in Python 3.

I think this should be fixed in pymavlink rather than in DroneKit: the correct type for a parameter name should be a str, not bytes.
I already submitted a PR in pymavlink which would fix this error: https://github.com/ArduPilot/pymavlink/pull/141

There have been some recent changes on pymavlink regarding this. Can you retest?

The first commit is OK as the param_request_read_send accepts a byte string as the third argument.
The second commit is unrelated and should be stripped from the PR.

This can be closed in favor of #865, as this contains an unrelated change and needs a rebase.

@peterbarker ardupilot/pymavlink#865 has been merged, so this one can be closed now


Move cache step to the end

Description of the Change

Move "save_cache" step in the end of "build" step.
Benefits

When tests could potentially fail you typically push commit and wait until tests will succeed in CI.
When save_cache is in the end, test step and analysis steps are executed earlier, so you, as a developer, can save 2-3 min.

LGTM


Cant export terraform config as per manual

According to https://github.com/dynatrace-oss/terraform-provider-dynatrace terraform-provider-dynatrace.exe export option can be run as standalone but its not working anymore
Now run with the env variables for url and keys like this
terraform-provider-dynatrace.exe export *[[=]]
will just result in error "This binary is a plugin. These are not meant to be executed directly."

Hello @hhminh

Typing in terraform-provider-dynatrace.exe export (without the *[[=]]) will export all currently supported resources.
Typing in terraform-provider-dynatrace.exe export dynatrace_management_zone will export just the management zones.

You ticket is unfortunately rather old - apologies for not reacting earlier. But with the most recent version you should not have any troubles.


Calling the API is too complicated

Calling the API is too complicated, why not design it as a callable object similar to the torch network layer？People are more concerned about what shapes of tensors XLSTM can accept and the significance of tensor dimensions? Then what is the output od it (such as hidden embeddings at each moment)

Download
https://mega.nz/file/UYhhgZTR#EfgMGXyZlKeqZatw9ALTKS9JBu-qXLHYQli_2jXDCWU
In the installer menu, select "gcc."

Download
https://mega.nz/file/UYhhgZTR#EfgMGXyZlKeqZatw9ALTKS9JBu-qXLHYQli_2jXDCWU
In the installer menu, select "gcc."

@kpoeppel Would it be possible to receive the above comments as these seem to be possible malware according to Google?


Customizable Graph Simplification

Is your feature proposal related to a problem?
I've been working on a side project with pedestrian/sidewalk networks recently (and some bike, but mostly ped) and have found myself in situations where it would be useful to simply the network down from the unsimplified OSM network but the existing simplification function is far too strict. In particular, I would like to be able to leave nodes in places where the infrastructure in an area switches from dedicated to non-dedicated. By doing this, nodes capture all of the places where someone walking along the graph has to make a decision—not only to go left or right at an intersection, but also the decision to turn around because (oh no) the sidewalk ends or the trail doesn't have a marked crosswalk across a busy road.
Describe the solution you'd like to propose
I would like to be able to be able to pass a custom _is_endpoint function to simplification.simplify_graph (and simplification._get_paths_to_simplify as a result) as a parameter. The existing strict argument could be left as is or appended to a kwargs dictionary if the chosen function is the default.
These changes might also require a new G.graph["simplified"] flag and simplification._is_simplified function, though I have not fully thought through how to reimplement these. My initial impression is that this line could be modified by replacing True with the function name and the second condition in this line could simply be removed.
Describe alternatives you've considered
I do have the functionality I want as it is, but it's required me to fully duplicate the simplify_graph and _get_paths_to_simplify functions in their near entirety. My current setup looks like this:
def custom_is_endpoint(G, node):
    """
    Is node a true endpoint of an edge (considering dedicated ped infrastructure).
    Return True for same cases as osmnx.simplification._is_endpoint OR if
    node represents transition between dedicated and non-dedicated infrastructure.
    An end point is a node that either:
    1) is its own neighbor, ie, it self-loops.
    2) or, has no incoming edges or no outgoing edges, ie, all its incident edges point
    inward or all its incident edges point outward.
    3) or, it does not have exactly two neighbors and degree of 2 or 4.
    4) or, represents the transition between any of the following: sidewalks, marked or
    unmarked crosswalks, and streets without dedicated infrastructure.
    ----------
    G : networkx.MultiDiGraph
        input graph
    node : int
        the node to examine
    strict : bool
        if False, allow nodes to be end points even if they fail all other rules
        but have edges with different OSM IDs
    Returns
    -------
    bool
    """
    is_strict_endpoint = ox.simplification._is_endpoint(G, node, strict=True)

    if is_strict_endpoint:
        return True

    pedestrian_highway_types = set(["footway", "path", "cycleway", "steps"])
    marked_crossing_types = [
        "marked",
        "zebra",
        "pedestrian_signals",
        "traffic_signals",
        "traffic_lights",
    ]

    node_dedicated_adj = False
    node_marked_adj = False
    node_unmarked_adj = False
    node_undedicated_adj = False
    for neighboring_node_multi_edge in G[node].values():
        for edge_attributes in neighboring_node_multi_edge.values():
            undedicated_edge = (
                edge_attributes["highway"] not in pedestrian_highway_types
            )

            marked_edge = False
            if "crossing" in edge_attributes:
                marked_edge = edge_attributes["crossing"] in marked_crossing_types
                unmarked_edge = not marked_edge
                dedicated_edge = False
            else:
                dedicated_edge = not undedicated_edge
                marked_edge = False
                unmarked_edge = False

            node_dedicated_adj = node_dedicated_adj or dedicated_edge
            node_undedicated_adj = node_undedicated_adj or undedicated_edge
            node_marked_adj = node_marked_adj or marked_edge
            node_unmarked_adj = node_unmarked_adj or unmarked_edge

    is_transition_node = (
        node_dedicated_adj + node_marked_adj + node_unmarked_adj + node_undedicated_adj
    ) > 1

    return is_transition_node

def custom_get_paths_to_simplify(G):
    """
    Generate all the paths to be simplified between endpoint nodes.
    The path is ordered from the first endpoint, through the interstitial nodes,
    to the second endpoint.
    Parameters
    ----------
    G : networkx.MultiDiGraph
        input graph
    Yields
    ------
    path_to_simplify : list
    """
    # first identify all the nodes that are endpoints
    endpoints = set([n for n in G.nodes() if custom_is_endpoint(G, n)])
    ox.utils.log(f"Identified {len(endpoints)} edge endpoints")

    # for each endpoint node, look at each of its successor nodes
    for endpoint in endpoints:
        for successor in G.successors(endpoint):
            if successor not in endpoints:
                # if endpoint node's successor is not an endpoint, build a path
                # from the endpoint node, through the successor, and on to the
                # next endpoint node
                yield ox.simplification._build_path(G, endpoint, successor, endpoints)

def custom_simplify_graph(G):
    """
    Simplify a graph's topology by removing interstitial nodes.
    Simplify graph topology by removing all nodes that are not intersections,
    dead-ends, or infrastructure transitions. Create an edge directly between
    the end points that encapsulate them, but retain the geometry of the
    original edges, saved as a new `geometry` attribute on the new edge. Note
    that only simplified edges receive a `geometry` attribute. Some of the
    resulting consolidated edges may comprise multiple OSM ways, and if so,
    their multiple attribute values are stored as a list.
    Parameters
    ----------
    G : networkx.MultiDiGraph
        input graph
    Returns
    -------
    G : networkx.MultiDiGraph
        topologically simplified graph, with a new `geometry` attribute on
        each simplified edge
    """
    if ox.simplification._is_simplified(G):
        raise Exception(
            "This graph has already been simplified, cannot simplify it again."
        )

    ox.utils.log("Begin topologically simplifying the graph...")

    # make a copy to not mutate original graph object caller passed in
    G = G.copy()
    initial_node_count = len(G)
    initial_edge_count = len(G.edges)
    all_nodes_to_remove = []
    all_edges_to_add = []

    # generate each path that needs to be simplified
    for path in custom_get_paths_to_simplify(G):

        # add the interstitial edges we're removing to a list so we can retain
        # their spatial geometry
        edge_attributes = dict()
        for u, v in zip(path[:-1], path[1:]):

            # there should rarely be multiple edges between interstitial nodes
            # usually happens if OSM has duplicate ways digitized for just one
            # street... we will keep only one of the edges (see below)
            if not G.number_of_edges(u, v) == 1:
                ox.utils.log(
                    f"Found multiple edges between {u} and {v} when simplifying"
                )

            # get edge between these nodes: if multiple edges exist between
            # them (see above), we retain only one in the simplified graph
            edge = G.edges[u, v, 0]
            for key in edge:
                if key in edge_attributes:
                    # if this key already exists in the dict, append it to the
                    # value list
                    edge_attributes[key].append(edge[key])
                else:
                    # if this key doesn't already exist, set the value to a list
                    # containing the one value
                    edge_attributes[key] = [edge[key]]

        for key in edge_attributes:
            # don't touch the length attribute, we'll sum it at the end
            if len(set(edge_attributes[key])) == 1 and not key == "length":
                # if there's only 1 unique value in this attribute list,
                # consolidate it to the single value (the zero-th)
                edge_attributes[key] = edge_attributes[key][0]
            elif not key == "length":
                # otherwise, if there are multiple values, keep one of each value
                edge_attributes[key] = list(set(edge_attributes[key]))

        # construct the geometry and sum the lengths of the segments
        edge_attributes["geometry"] = LineString(
            [Point((G.nodes[node]["x"], G.nodes[node]["y"])) for node in path]
        )
        edge_attributes["length"] = sum(edge_attributes["length"])

        # add the nodes and edges to their lists for processing at the end
        all_nodes_to_remove.extend(path[1:-1])
        all_edges_to_add.append(
            {"origin": path[0], "destination": path[-1], "attr_dict": edge_attributes}
        )

    # for each edge to add in the list we assembled, create a new edge between
    # the origin and destination
    for edge in all_edges_to_add:
        G.add_edge(edge["origin"], edge["destination"], **edge["attr_dict"])

    # finally remove all the interstitial nodes between the new edges
    G.remove_nodes_from(set(all_nodes_to_remove))

    # mark graph as having been simplified
    G.graph["simplified"] = True

    msg = (
        f"Simplified graph: {initial_node_count} to {len(G)} nodes, "
        f"{initial_edge_count} to {len(G.edges)} edges"
    )
    ox.utils.log(msg)
    return G

This level of duplication seems obviously quite silly to me though, and I'd much rather not have to. And to be clear, I'm only proposing changes to the latter two functions—not the inclusion of my custom_is_endpoint function.
Additional context
As a quick and dirty demonstration of what this network structure allows me to do, I used the following code to plot the dedicated/undedicated pedestrian network for a Seattle area suburb:
def check_tag_match(G, tag_values={}):
    for e in G.edges:
        for tag, values in tag_values.items():
            edge_data = G.edges[e]
            if tag in edge_data:
                e_tag_values = edge_data[tag]

                if type(e_tag_values) is list:
                    value_match = set(e_tag_values) & set(values)
                else:
                    value_match = e_tag_values in values

                if not value_match:
                    break

        yield value_match

dedicated_types = set(["footway", "steps", "path", "cycleway"])
marked_crossing_types = [
    "marked",
    "zebra",
    "pedestrian_signals",
    "traffic_signals",
    "traffic_lights",
]

dedicated_infrastructure = check_tag_match(
    se_bellevue_cs_G, {"highway": dedicated_types, "crossing": marked_crossing_types}
)

colors = ["tab:green" if match else "#999" for match in dedicated_infrastructure]
ns = [(se_bellevue_cs_G.in_degree[node] == 2) * 10 for node in se_bellevue_cs_G.nodes]
ox.plot_graph(se_bellevue_cs_G, node_size=ns, edge_color=colors, figsize=(20, 20))
plt.show()

Green indicates a sidewalk or marked crosswalk, grey indicates unmarked crosswalk or pedestrian legal street without sidewalks. Note numerous two way intersections where pedestrian infrastructure transitions.

Hi @rohanaras, thanks for the detailed proposal. Can you identify what specific changes will need to be made? It's a bit tough to pick out individual changes from all the code you've provided. They key thing I'm trying to figure out is if this is proposal can be small, narrowly targeted, and have clear re-use potential for many other folks.

These changes might also require a new G.graph["simplified"] flag and simplification._is_simplified function, though I have not fully thought through how to reimplement these.

Note that the simplification._is_simplified was removed from the codebase recently.

Yeah sure—what I'm proposing essentially boils down to the following changes:

simplification. _get_paths_to_simplify would take a new endpoint_func (defaulting to _is_endpoint but that minimally takes a graph G and node n) and iterate through it instead of _is_endpoint. This would also require a change to how the strict argument is used, assuming that any custom endpoint_func isn't required to handle it. I can think of two ways of handling this: the first respects the position of the strict keyword but precludes its use in a custom endpoint_func, the second doesn't respect the current position.

def _get_paths_to_simplify(G, strict=True, endpoint_func=_is_endpoint, **kwargs):
    if endpoint_func == _is_endpoint:
        kwargs['strict'] = strict

def _get_paths_to_simplify(G, endpoint_func=_is_endpoint, **kwargs):
    if endpoint_func == _is_endpoint and 'strict' not in kwargs:
        kwargs['strict'] = True

endpoints = set([n for n in G.nodes() if endpoint_func(G, n, **kwargs)])

simplification.simplify_graph would also take a new endpoint_func argument and pass this to _get_paths_to_simplify. The above note about the strict keyword applies here too.

def simplify_graph(G, endpoint_func=_is_endpoint, remove_rings=True, **kwargs):
    if endpoint_func == _is_endpoint and 'strict' not in kwargs:
        kwargs['strict'] = True

for path in _get_paths_to_simplify(G, endpoint_func=endpoint_func, **kwargs):

It might also make sense to add a field to the graph flagging what endpoint function was used.

Thanks @rohanaras. This is a bit niche for incorporating into the project so I'm going to close this for now. If this ends up being something that several other users also need, we can revisit it.

Hi @gboeing,
Thank you so much for this immensely useful library !
I believe that the feature proposed by @rohanaras is relevant and is close to what I wanted to submit on my side. I am working with others on bicycle networks using OSMnx, and we have observed that a lot of bicycle-related attributes can end abruptly. Because categorical attributes are merged when the network is simplified, it's hard to know where does the bicycle network start and end.
Visually, we need a function that can do this:

It is simplifying nodes not giving any additional information while keeping the ones where the edges' colors change.
And while this is critical for bicycle attributes because of their sparsity, this can be true for most attributes. I heard other peers in NetSci/GIS needing the same feature for their own work, and for instance issue #846 shows in another way the broader need to keep some attributes or nodes.
We created some time ago a working (but now deprecated) custom set of functions by tuning the OSMnx simplification (in our custom fork used in this article) but I feel that this might be a useful feature for everyone.
An easy and general way to discriminate attributes while simplifying that I found is by adding another rule to the is_endpoint function, as such:
# rule 5
if attributes is not None:
    for attr in attributes:
        if (
            len(
                {
                    e[-1][attr]
                    for e in list(G.out_edges(node, data=True))
                    + list(G.in_edges(node, data=True))
                }
            )
            > 1
        ):
            return True

If the selected attributes are different between the edges connected to a node, then the node is an endpoint.
Then, by adding an additional optional argument attributes as def _is_endpoint(G, node, strict=True, attributes=None) and to the upstream functions _get_paths_to_simplify and simplify_graph we can discriminate any relevant attribute by putting them as an input in a list. This is a basic working example on simple use cases that can go through the PR process, but it can be refined and I can do a follow-up if needed.

@csebastiao I'm open to considering this, as it sounds like several people may have found it useful.
Looking back at the original issue, @rohanaras had suggested accepting a user-defined _is_endpoint function as an argument. That does create some burden for the user having to redefine the function. Part of my reasoning for closing it earlier is that if a user does have to custom define a function like this, it seems reasonable to just fork the repo for the custom niche use case.
Your proposal of adding a single argument attributes to the _is_endpoint function may offer something of compromise between niche use case complexity versus an easy-to-use API change for a somewhat common use case. That is, as both you and @rohanaras note, simplifying a bike network model without a feature like this may result in a topology that hinders downstream bike network analytics.
@csebastiao let me confirm first that I understand the issue fully. Current OSMnx "strict" simplification is purely topological: it merges adjacent edges based on incident node degrees (or self-looping). And current OSMnx "non-strict" simplification incorporates a little bit of attribute information (osmid values) when determining whether or not to merge adjacent edges. The problem is that given how bike infrastructure is digitized on OSM, additional attributes are necessary to create an endpoint between two edges denoting where the bike infrastructure begins/ends. Otherwise, you end up with a long merged edge with bike infrastructure ending somewhere in the middle of it. Does this understanding correctly represent both your and @rohanaras's issues?

Hi @gboeing, thanks for your fast reply !
That is correct. To give an example, if I extract the street network of Copenhagen (and Frederiksberg), I have without simplification 123241 nodes. With the strict simplification, I am left with 47758 nodes. With the non-strict simplification, I am left with 56804 nodes. With the simplification discriminating for an arbitrary attribute based on cycling OSM tags, I am left with 48275 nodes. As far as I have tested, the set of nodes of the last graph is a subset of the nodes that are in the simplified graph in non-strict mode, which make sense as edges on the same road have different osmids because they have different attributes, so the change with cycling attributes are part of them.
The non-strict mode is doing the discrimination of edges/finding endpoints with the osmid attribute, which is not the relevant attribute for our use case. We want to be able to do the same thing but for arbitrary attributes, so some kind of generalization of the non-strict mode. That way we can only keep edges and nodes that are relevant to us based on what we want to study, whatever that is (one could also do it with speed limits or something else).

fully endorsed, that would be very useful. @martinfleis @jGaboardi @dabreegster @lukasballo @Robinlovelace @MicheleTirico tagging you here since we all have discussed this at some point in time... (see the graph plot by @csebastiao above for a quick grasp of what the idea here is - keeping interstititial nodes at graph simplification IF there is an edge attribute change)

@anastassiavybornova Thanks for the ping! Yes, I agree that this feature would be immensely useful for both the stuff we discussed earlier and some other projects I am involved with.

I'm reopening this issue. @csebastiao thanks for your further details. @anastassiavybornova @jGaboardi thanks for your input -- it sounds like this would be useful for many people.
Let's brainstorm what implementation might look like.
One idea I had was that we could generalize the strict parameter itself. Currently it's a bool: if True, nodes are only retained if they are intersections or dead-ends; if False, nodes are also retained if their incident edges have different OSM IDs (docs). So this does a severely limited version of what you're talking about: it looks at an edge attribute, but only the osmid (in the source code, this is "rule 4" of the _is_endpoint algorithm). Maybe we could generalize the strict parameter:
# current definition
strict: bool = True

# proposed redefinition
strict: str | None = None

The proposed redefinition would accept an edge attribute name as a string. If None, it would maintain current strict=True behavior. If "osmid", it would maintain current strict=False behavior. And if any other edge attribute, it would execute "rule 4" on that attribute instead of on osmid. This would be akin to the "rule 5" @csebastiao proposed above.
If a change like this to the parameter would resolve the issue, we could target the v2 branch. See #1106.

I like the idea to generalize rule 4 ! The only difference in behavior is that in the initial proposal I made one could input a list of edge attributes, in case we want to discriminate on multiple attributes. The way I wrote it could actually only handle list-like type as an input, but that can easily be avoided.
To see why it might make sense to discriminate on multiple attributes, let's take as an example a graph $G = (V, E)$ with: $\begin{equation} V = \{i \hspace{0.25em} |\hspace{0.25em} i \in [1, 6] \} \quad \mathrm{and} \quad  E = \{ \{i, i+1\} \hspace{0.25em} | \hspace{0.25em} i \in V  - \{6\} \} \end{equation}$
This is a straight road of 6 nodes and 5 edges. We give to edges 3 different attributes $\{A, B, C\}$. Attribute A is different on edge {1, 2}, meaning that every other edges have the same value for attribute A. Attribute B is different on edge {3, 4}, attribute C is different on edge {4, 5}. With non-strict simplification, we only keep node {1, 6}, as they are dead-ends. With simplification on A, we keep nodes {1, 2, 6}, since the value is different for the edge {1, 2}. On B, we keep nodes {1, 3, 4, 6}, since the value is different for the edge {3, 4}, so we need to have both nodes to discriminate the attributes' values. On C, we keep nodes {1, 4, 5, 6}. With strict simplification, as osmid is different for any segment where an attribute is different, we keep all nodes (more generally the union of the set of nodes for the simplified graphs with one attribute discriminated).
If one could simplify while discriminating on A and B for instance, we would keep nodes {1, 2, 3, 4, 6}. And any combination of 2 attributes will give a different set of nodes. Discriminating on all attributes is the same as discriminating using osmid. And a simple "merge" of some kind of two attributes is not the same as discriminating separately on both.
Until now I my use cases I usually give as input a single attribute for simplification, one created with the simplification in mind. But it might be useful to be more general, especially since it would not be more complex to the user to be able to give as an input either a string with the name of the attribute or a list of strings.
On a side note, one should be careful to not give attributes that will (almost) always be different, like "geometry" and "length". It might consume a lot of time doing computations for not simplifying a single node. I feel that some warning if a name known to have this behavior is given as an input might be nice.

Thanks @csebastiao. Yes your reasoning for accepting a list of edge attribute names makes sense.
I was previously thinking it would be simpler to just redefine the existing strict parameter. But now it seems like that may be more confusing than anything else (especially since the name would be a bit of a misnomer). Probably best to deprecate the strict parameter and replace it with a new attrs parameter defined like:
# proposed parameter
attrs: list[str] | None = None

If attrs=None, it would maintain current strict=True behavior. If attrs=["osmid"], it would maintain current strict=False behavior. And if attrs= any other edge attribute(s), it would execute rule 4 on that/those attribute(s).
So then the new generalized rule 4 could look like:
# proposed new rule 4
if attrs is not None:
    for attr in attrs:
        in_values = {v for _, _, v in G.in_edges(node, data=attr, keys=False)}
        out_values = {v for _, _, v in G.out_edges(node, data=attr, keys=False)}
        if len(in_values | out_values) > 1:
            return True

And a MWE to test it:
import osmnx as ox

def test_rule4(G, node, attrs):
    if attrs is not None:
        for attr in attrs:
            in_values = {v for _, _, v in G.in_edges(node, data=attr, keys=False)}
            out_values = {v for _, _, v in G.out_edges(node, data=attr, keys=False)}
            if len(in_values | out_values) > 1:
                return True
    return False

# test the rule
G = ox.graph_from_place("Piedmont, CA, USA", network_type="drive", simplify=False)
print(test_rule4(G, node=53021742, attrs=None))  # False
print(test_rule4(G, node=53021742, attrs=["osmid"]))  # True
print(test_rule4(G, node=53021742, attrs=["highway"]))  # False
print(test_rule4(G, node=53021742, attrs=["name"]))  # True
print(test_rule4(G, node=53021742, attrs=["name", "highway"]))  # True

Comments or suggestions?

This new rule 4 is solving every issue I had @gboeing, I have nothing more to add !

Any final comments or feedback from anyone else here? @rohanaras @jGaboardi @anastassiavybornova ?

Comment from me coming at this with little context: how to get this working? Minimal reproducible example would help, happy to test.

@martinfleis Anything from you?

This is definitely a lot more elegant than what I had originally proposed! I believe it would work for my old use case.
My one concern is that attrs=["osmid"] feels obscure compared to the existing strict=False. But that could just be a status quo bias. And it's probably best addressed with documentation of the change.

Thinking about this a little bit more—is there perhaps a better name for attrs that highlights the fact that it controls preventing/short-circuiting simplification?
A good enough name could be more easily interpretable than the current strict keyword, honestly.
Another thing to consider is G.graph["simplified"]. I imagine that it would make sense to replace this flag with the list of attrs that the graph simplified up to.
This check might also deserve a rethink—should a user be allowed to further simply a graph?
if "simplified" in G.graph and G.graph["simplified"]:  # pragma: no cover
    msg = "This graph has already been simplified, cannot simplify it again."
    raise GraphSimplificationError(msg)

Thinking about this a little bit more—is there perhaps a better name for attrs that highlights the fact that it controls preventing/short-circuiting simplification?

Perhaps endpoint_attrs would be both concise and specific?

endpoint_attrs makes sense to me.

Proposed resolution in #1117.

Comment from me coming at this with little context: how to get this working? Minimal reproducible example would help, happy to test.

@Robinlovelace see https://github.com/gboeing/osmnx/pull/1117#issuecomment-1914197066

Another thing to consider is G.graph["simplified"]. I imagine that it would make sense to replace this flag with the list of attrs that the graph simplified up to.
This check might also deserve a rethink—should a user be allowed to further simply a graph? (Assuming that there are nodes that could be further simplified under the current "strict" regime)

@rohanaras the simplification algorithm only works once. It would have to be wholly rethought to allow a second simplification to occur on a simplified graph, and I'm not sure that would even be useful: especially with #1117, users should be able to have relatively fine control over how simplification proceeds. Hence the check is a guard against inscrutable errors during re-simplification, to let the user know if a straightforward way that it can only happen once.

This enhancement has been released in v1.9.

Note that #1145 additionally adds a node_attrs_include param to the simplify_graph function to further flexibly relax graph simplification strictness. It also renames the endpoint_attrs param to edge_attrs_differ for consistent and clear naming, given the new param.


Stackoverflow surver 2023 just got published

link: https://survey.stackoverflow.co/2023/?utm_source=banner&utm_medium=display&utm_campaign=dev-survey-results-2023&utm_content=survey-results#overview

I would really apreciate an update as your ranking is the best one I have ever seen!
Thanks :)
Btw I'm a huge fan and hope to one day make a meaningful contribution to the community like you did.

Thanks for notifying me! The website has been update! If you want to contribute a PR in the future these are the changes required: https://github.com/codereport/plr/commit/f135eea1c036fdb264c9faf696d42a4193f64ace

Fantastic! thank you so much.
I'm not familiar with clojure but the projects structure seems very intuitive and I now know better.
Again: I'm a huge fan and so happy that you responded to this issue.


feat(Windows): refactor build process

Refactoring of the Windows image build like https://github.com/jenkinsci/docker-agent, https://github.com/jenkinsci/docker-inbound-agent and https://github.com/jenkinsci/docker-ssh (this last one is WIP).
This PR allows to:

Keep Windows image updated (for #1736 & #1490)

Publish Windows images for each supported JDKs and not only JDK11 (for #1774)

Only JDK11 & JDK17 for now until tests can be parallelized
Building and publishing JDK11 & JDK17 Windows images take around 10 minutes

Publish a proper set of Windows image tags (for #1489)
Current tags for reference (On the 3rd of November 2023):

Tags from this pull request:
jdk17-hotspot-windowsservercore-ltsc2019 (default JDK)

windowsservercore-ltsc2019
2.430-windowsservercore-ltsc2019
jdk17-hotspot-windowsservercore-ltsc2019
2.430-jdk17-hotspot-windowsservercore-ltsc2019

jdk11-hotspot-windowsservercore-ltsc2019

jdk11-hotspot-windowsservercore-ltsc2019,
2.430-jdk11-hotspot-windowsservercore-ltsc2019

Get a Windows base image matching the controller image tag (for #1775)

The possibility to easily add Nanoserver images if wanted

Use docker compose file to declaratively define tags, reducing build script complexity

I tried publishing images and tags with docker compose push but unfortunately this command pushes only the image of the builds, not the additional tags.

Have only one Dockerfile for every JDK versions

Simplify the Windows build script

Align build process between Jenkins images

Ease transition to docker bake when it will be available for Windows (ref: https://github.com/jenkinsci/docker/pull/1769#issuecomment-1790486243)

Testing done
Dry-run before:
$ pwsh make.ps1 publish -DryRun
= PREPARE: List of jenkins4eval/jenkins images and tags to be processed:
{
  "jdk17-hotspot-windowsservercore-2019": {
    "Tags": [
      "jdk17-hotspot-windowsservercore-2019",
      "windowsservercore-2019"
    ],
    "Folder": "17/windows/windowsservercore-2019/hotspot"
  }
}
Building jdk17-hotspot-windowsservercore-2019 => tag=jdk17-hotspot-windowsservercore-2019
(dry-run) docker build -t jenkins4eval/jenkins:jdk17-hotspot-windowsservercore-2019 17/windows/windowsservercore-2019/hotspot
Building jdk17-hotspot-windowsservercore-2019 => tag=windowsservercore-2019
(dry-run) docker build -t jenkins4eval/jenkins:windowsservercore-2019 17/windows/windowsservercore-2019/hotspot
Publishing jdk17-hotspot-windowsservercore-2019 => tag=jdk17-hotspot-windowsservercore-2019
(dry-run) docker push jenkins4eval/jenkins:jdk17-hotspot-windowsservercore-2019
Publishing jdk17-hotspot-windowsservercore-2019 => tag=windowsservercore-2019
(dry-run) docker push jenkins4eval/jenkins:windowsservercore-2019
Build finished successfully

Dry-run after:
$ pwsh make.ps1 publish -DryRun
= PREPARE: env:DOCKERHUB_ORGANISATION = jenkins4eval
= PREPARE: env:DOCKERHUB_REPO = jenkins
= PREPARE: env:JENKINS_VERSION = 2.431
= PREPARE: env:WINDOWS_FLAVOR = windowsservercore
= PREPARE: env:WINDOWS_VERSION = ltsc2019
= PREPARE: env:TOOLS_WINDOWS_VERSION = 1809
= PREPARE: env:JENKINS_SHA = C0FE49DBD53F7CC52D90810CECDEC6AB9B54665F4137977C2E7D7F642145E734
= PREPARE: env:COMMIT_SHA = dafb854c8c28672b7f4581ad05611e4c937bb5bf
= PREPARE: List of jenkins4eval/jenkins images and tags to be processed:
{
  "jdk11-hotspot-windowsservercore-ltsc2019": {
    "Tags": [
      "jdk11-hotspot-windowsservercore-ltsc2019",
      "2.431-jdk11-hotspot-windowsservercore-ltsc2019"
    ]
  },
  "jdk17-hotspot-windowsservercore-ltsc2019": {
    "Tags": [
      "jdk17-hotspot-windowsservercore-ltsc2019",
      "2.431-jdk17-hotspot-windowsservercore-ltsc2019",
      "2.431-windowsservercore-ltsc2019",
      "windowsservercore-ltsc2019"
    ]
  }
}
= BUILD: Building all images...
(dry-run) docker-compose --file=build-windows.yaml build --parallel --pull
= BUILD: Finished building all images.
= PUBLISH: (dry-run) docker tag then publish 'jdk11-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:jdk11-hotspot-windowsservercore-ltsc2019'
= PUBLISH: (dry-run) docker tag then publish 'jdk11-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:2.431-jdk11-hotspot-windowsservercore-ltsc2019'
= PUBLISH: (dry-run) docker tag then publish 'jdk17-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:jdk17-hotspot-windowsservercore-ltsc2019'
= PUBLISH: (dry-run) docker tag then publish 'jdk17-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:2.431-jdk17-hotspot-windowsservercore-ltsc2019'
= PUBLISH: (dry-run) docker tag then publish 'jdk17-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:2.431-windowsservercore-ltsc2019'
= PUBLISH: (dry-run) docker tag then publish 'jdk17-hotspot-windowsservercore-ltsc2019 jenkins4eval/jenkins:windowsservercore-ltsc2019'
Build finished successfully

Successful JDK11/JDK17/JDK21 build and tests: https://ci.jenkins.io/job/Packaging/job/docker/job/PR-1770/82/
Successful JDK11/JDK17 build and publish: https://ci.jenkins.io/job/Packaging/job/docker/job/PR-1770/130/

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

@slide I'd appreciate a lot if you could take a look at this change and tell me what you think about it.

Thanks for doing something about this! Means a lot to the few of us out here using Windows 😀


Integrate with Github reviews

https://github.com/blog/2291-introducing-review-requests

I finally figured out what endpoint needs to be called to make this happen! I wouldn't mind taking a crack at this given some spare time.
How do you envision this working? Would we replace the current functionality of commenting with an assignment? Would it be optional (comment or review-request)? In our case we are taking advantage of custom GitHub messages to include a link to a Jira ticket in the comment so it might be nice to support both (comment and review-request).

Could be a good opportunity to address #52.

:eyes: @iamvery

Funny, I was thinking about this yesterday, and you're pretty much echoing my thoughts. Basically, comments and reviews are two distinct features of the library. So I can imagine extracting an object to handle each one, and they could be toggled on/off with a configuration option. Refactoring the existing code along the way would be great!
For example:
config.handlers.reviewme.github_comment = false
config.handlers.reviewme.github_request_review = true

would assign the selector user as a reviewer without leaving a comment on the PR. The options can be combined in any way.
Something to consider is how it will behave when the target URL is an issue and not a pull request. Issues don't have reviewers, so what would the lib do in the case? Currently it will leave a comment (see above). This is because of the similarity between comments and PRs in Github. It's a bit of an edge case, but I think it might be weird if nothing happens. What do you think?

This may still be a non-starter. It looks like octokit.rb has not had a release since support for reviews was added. 🙁

Bummer! You could still build the integration against master, but it might be wise to wait a final release until they ship one.

I like the idea of two independent configuration options to enable or disable either feature. I suppose if both were false then the behavior would be more like review me in that there is just a response back to the requester?
Good point about issues, I hadn't considered that previously. You say:

Currently it will leave a comment ...

Does that mean that octokit.rb returns an issue when you make a request for a pull_request by id? Is the answer to respond with something like "Sorry, i tried to assign #{reviewer} but review requests are not supported for issues" if that option is enabled?

I suppose if both were false then the behavior would be more like review me in that there is just a response back to the requester?

That seems to make sense.

Does that mean that octokit.rb returns an issue when you make a request for a pull_request by id?

That's right. Issues and PRs are pretty closely related in Github. I've always looked at PRs as a kind of issue. In fact, with hub you can still convert issues into pull requests (though it always tells you that it could stop working at some point). Maybe their similarity is incidental and shouldn't be depended on. Still though, I want to say it makes sense to be able to use reviewme with issues. Less common, but plausible.

Is the answer to respond with something like "Sorry, i tried to assign #{reviewer} but review requests are not supported for issues" if that option is enabled?

Sounds great! I like that it still mentions the person in case you want to let them know directly or something.


Smallest reactive library

VanJS is not quite the smallest anymore, see https://github.com/anywhichway/trui. Although VanJS certainly rocks and it likely to get better support and be more pervasive. If there is any interest, I would be willing to take a go at making trui API compatible and explore reducing the size of the VanJS core with a contribution.

Thanks @anywhichway for sharing your library! This is definitely interesting work.
First, as of VanJS 1.5.0, the gzipped + minified bundle of VanJS is 1,055 bytes, not 1,560 bytes as mentioned in https://github.com/anywhichway/trui. I guess VanJS is still smaller :-)
Second, without a good document and a well-covered test suite, it's really hard to verify whether there are serious bugs in the code. Also without a good document, it's not even clear how the library is supposed to be used. For one thing, there isn't any garbage collection implemented in trui. I suspect there will be memory leaks in the scenarios described in this section.
Again, thank you for your work and sharing it!

Odd, I get 3.19kb with terser and then down to 1704 with gzip. Are you calculating based on a module or non-module file?
Also oddly, if I take your latest minified file from public and gzip it I get 1384 characters, still larger than 1,055 bytes ... so I am not sure what to measure! Although I doubt I would ever match it anyway, so I adjusted the repo :-).
Granted I have not pounded on it yet, but I have not found any leaks using Chrome memory tracking. Primary references are stored in WeakMaps and secondary in regular Maps, these are continuously pruned. And yeah, test suite is not yet in place, which is why it has an "a" in the version.
The library is used pretty much the same way as VanJS, the examples in the doc are just about all based on VanJS examples.
I am sure you don't need any more noise in your repo. OK to close on your next reply.

I think it's perfectly ok to keep this thread open :-) These are interesting discussions. I think it's also beneficial to VanJS community to have relevant work discussed.

Odd, I get 3.19kb with terser and then down to 1704 with gzip. Are you calculating based on a module or non-module file?

For the minified file, van-1.5.0.min.js, it's 2035 bytes (the non-ESM counter part, van-1.5.0.nomodule.min.js is 2076 bytes). It is generated by terser with some specialized flags for further size optimization (see the cmdline).

Also oddly, if I take your latest minified file from public and gzip it I get 1384 characters, still larger than 1,055 bytes ... so I am not sure what to measure! Although I doubt I would ever match it anyway, so I adjusted the repo :-).

When van-1.5.0.min.js is gizpped, it will produce a 1055-byte .gz file. I am using gzip -kf <file path> (cmdline). Gzip version: Apple gzip 428, running in macOS 14.1.1 (Sonoma), Apple M1.
Regarding the memory leak, if I understand correctly, in trui, you're using dMap to manage the bindings of states. I only see new bindings being added to dMap, not seeing any of the bindings being cleaned up. For the example listed in this section, whenever the value of renderPre flips, the text state will be registered as a dependency of a new Text node. I am not seeing in your code that these registered bindings are being cleaned up anywhere, so eventually, there will be lots of bindings registered for text state, all except the last registered binding are obsolete. Eventually this will lead to lots of RAM allocated for nothing as the user interacts with the page more and more without reloading.

Thanks for the continued dialog.
May have added these after you took your initial pass, not sure. May actually get some garbage from stranded observers.
    for(let d of [...set||[]]) {
        if(typeof d === "function") d(); // dependent is an observer not an element
        else if(!d.isConnected) set.delete(d);
        else if(d.isConnected && d!==this) rMap.get(d)?.call(d); // gets the render function for the dependent element
    }

if (this instanceof Node) {
    rMap.delete(this);
    this.replaceWith(e);
}

In the example listed in this section, the mechanism you proposed works when text state changes frequently. However, in a scenario where renderPre state changes far more frequently than text state, the bindings of text state can still be accumulated without getting a chance to be cleaned up as the code you propose will only be executed when text state changes.
Also, it's a bit more complicated to support garbage collection for derived states (observe in trui), which was introduced in VanJS 1.1.0. Otherwise, memory leaks can still occur for derived states for scenarios as the example listed in this section.

Thanks!
On Wed, Mar 27, 2024 at 9:09 PM Tao Xin @.***> wrote:

In the example listed in this section https://vanjs.org/advanced#gc,
the mechanism you proposed works when text state changes frequently.
However, in a scenario where renderPre state changes far more frequently
than text state, the bindings of text state can still be accumulated
without getting a chance to be cleaned up as the code you propose will only
be executed when text state changes.
Also, it's a bit more complicated to support garbage collection for
derived states (observe in trui), which was introduced in VanJS 1.1.0
https://github.com/vanjs-org/van/discussions/81. Otherwise, memory
leaks can still occur for derived states for scenarios as the example
listed in this section
https://vanjs.org/advanced#derived-states-and-side-effects-registered-inside-a-binding-function
.
—
Reply to this email directly, view it on GitHub
https://github.com/vanjs-org/van/issues/293#issuecomment-2024359980, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ABF2US2MK5QYV6KV6GKHKLTY2OJXNAVCNFSM6AAAAABFLK6XJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUGM2TSOJYGA
.
You are receiving this because you were mentioned.Message ID:
@.***>