[Rich Text]- Problem with bullets and number list

Category
[ ] Enhancement
[x] Bug
[ ] Question
Version
Please specify what version of the library you are using: [ 2.3.0 ]
Expected / Desired Behavior / Question
Bullets and Number list should work properly.
Observed Behavior
When I'm using Number list first time then it works fine. But If I'll write some more text or delete the previous text and try to use the number list second time on the same webpart then it doesn't work. I have to select bullets first and then number list.
Similar case is happening if I'm selecting bullets first instead of number.

Steps to Reproduce
Add new webpart
Write some text and give numbering or bullets. It will work for the first time
Delete the text and again try to give the same you've given earlier either numbers or bullets
OR
Type some more text like I did in the above screenshot and try to give the same you've given earlier either numbers or bullets. It won't work.
Thanks!

Thank you @pk091990 for reporting this one!
The fix has been implemented and will be included in the next release.
In a meantime you can test it in beta version

Hi @AJIXuMuK ,
Mentioned issue has been fixed in the recent published 2.6.0 version. However notice another issue with rich text in this version . It was working fine with earlier version i.e. 2.3.0
Mentioning repro steps here. Let me know if you are able to repro it and I need to raise a separate issue for the same:
1: Add Rich text webpart
2: Copy paste any text and click outside of your webpart.
3: Text will disappear with below error in console.

Debug:

4: Its working fine if I'm manually writing any text instead of copy/paste.

Hi @pk091990 - could you please open a separate issue for this one?

Hi @pk091990 - could you please open a separate issue for this one?

Raised here

In html file, i have added this tag<ejs-richtexteditor #toolsRTE id='alltoolRTE' [(value)]='contentValue' [toolbarSettings]='tools'>
        </ejs-richtexteditor>.

content value has email template info
I am trying to use bullet list and number list  . I place the cursor on 7th line, and click o bullet list, but i see the bullet is showing on first line, same with the number


[1.20.1 Forge] Locate structure does not autocomplete

however locate biome does

debug.log
latest.log

Oh wow, that's surprising.
Seems like either the locate structure command is doing something weird or my mod is crashing/failing silently while sorting the suggestions.
I'll investigate it later, thanks for the report ^^

Alright, the difference between /locate biome and /locate structure was that the suggestions for /locate biome were being cached in the client, while those for /locate structure were being actually requested to the server (even in singleplayer).
The vanilla handling for those requests was discarding all except the last one, so my way of sending two of them at the same time caused one of them to be discarded, and the completions to wait forever.
I've fixed it in the latest version (1.1.0), which should be already available on CurseForge.
If you have the time, please confirm that the fix works for you.

verified fixed! thanks so much!


add note support to flags

Description
Related to issue #230
Adds a new notes section to the flag config view and updates the API to allow for adding a flag with attached notes in markdown format. Notes can be used for adding details about flag usage, describing the flag's purpose, adding examples of usage, etc.
Collapsed view when no notes are present:

Editor view:

View when notes present:

Motivation and Context
This solves the inconvenience of having to have separate, hard-to-find documentation pages for a given flag. Instead, all relevant documentation is held on the flag config page itself.
How Has This Been Tested?
Manual UI testing and updated CRUD tests to check for new field acceptance.
Types of changes

[ ] 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 change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

Codecov Report

Merging #243 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
+ Coverage   82.56%   82.59%   +0.02%
==========================================
  Files          25       25
  Lines        1440     1442       +2
==========================================
+ Hits         1189     1191       +2
  Misses        192      192
  Partials       59       59

Impacted Files
Coverage Δ

pkg/entity/flag.go
94.28% <ø> (ø)
:arrow_up:

pkg/handler/crud.go
91.04% <100%> (+0.05%)
:arrow_up:

Continue to review full report at Codecov.

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

@zhouzhuojie I moved the notes editor into the flag card as you suggested. It looks much better now. I also added some margin on the toggle button and made it dynamic based on editor mode. Still don't think it's worth messing with the snapshots for autosave functionality so I didn't add that back.
Editor closed:

Editor open:


Bypass Google Authentication for '/experiments' and '/metric' Endpoints

Is your feature request related to a problem? Please describe.
The api/experiments and api/metric/save endpoints are being requested by the demo app and the tester app, but they currently fail with a 401 (Unauthorized) error due to lack of Google authentication in the request.
I believe these endpoints are useful for testing UpGrade, and requiring Google authentication makes it difficult to use them. If we are concerned about the security, I think we can require different authentication methods, like an API key for developers.
Describe the solution you'd like
The @Authorized() decorator for these endpoints should be commented out (search for @JsonController('/experiments') and @JsonController('/metric') in the codebase) to bypass the Google authentication until we implement a different authentication method.
Describe alternatives you've considered
Implement different authentication methods, like an API key for developers.

I believe the person who owns the GOOGLE_CLIENT_ID we use for UpGrade needs to create a service account at https://console.cloud.google.com/, then create a service key JSON file and share the file with me through pwpush. See this Slack post for your reference.
@VivekFitkariwala @ppratikcr7 @danoswaltCL @bcb37

I believe the person who owns the GOOGLE_CLIENT_ID we use for UpGrade needs to create a service account at https://console.cloud.google.com/, then create a service key JSON file and share the file with me through pwpush (so that I can generate the access token for API requests). See this Slack post for your reference.
@VivekFitkariwala @ppratikcr7 @danoswaltCL @bcb37

I will do this for you. :)


Intel® Python Scikit-learn Extension Getting Started sample readme update

Existing Sample Changes
Description
Restructured to match new template. Removed mentions of Windows and macOS support based on build information found in the sample.json file. Removed jupyter-related images since they reference a different sample and don’t help with understanding. Moved other images to “images” directory. Rewrote some text for clarity. Fixed spelling errors. Fixed formatting. Updated branding per guidelines. Changed sample name in readme to match the name in the sample.json file. Renamed readme from “readme.md” to “README.md” to match GitHub standard practice.
Fixes Issue#
External Dependencies
N/A
Type of change
Please delete options that are not relevant. Add a 'X' to the one that is applicable.

[x] Documentation

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

[x] VSCode

Adding @jimmytwei for notices.


AWS ELBs idle timeout too short

The CloudFormation infrastructure defaults to 60 seconds for the ELB idle timeout. This causes disconnects when using oc port-forward and oc rsh. Please consider increasing the default idle timeout to something higher (maximum 3600 seconds) and perhaps include documentation regarding it.

 OpenShift-Ansible-Contrib issues and PRs

Thanks! However, it looks like a quote is missing on the right of each "IdleTimeout:" key.

The change was picked up in later commits. Should be good to go once its merged.


fix: use correct Rust variant name for enum aliases

This change fixes code generation of protobuf enums when aliases are present.

Closed by https://github.com/influxdata/pbjson/pull/103

Won't #103 just ignore the aliases in deserialize? This seems incomplete.

You can define aliases by assigning the same value to different enum constants. To do this you need to set the allow_alias option to true, otherwise the protocol compiler generates a warning message when aliases are found. Though all alias values are valid during deserialization, the first value is always used when serializing.

That is a valid point, I would be happy to review a PR to address this aspect and add a corresponding test for it

This PR addressed addressed that aspect, but it didn't add an explicit test.
Unfortunately, I am having trouble getting the CLA signed, so it might be better to reimplement this (small) change.

I can take a look at this gap in a bit.


Throwing error for a specific field

This is more of a discussion than an issue, but since discussions are not enabled here, I'm posing as an issue.
I am not sure if this is something I missed in the documentation, or we don't have this feature, but I can't find a way to throw an error for a specific field.
In my case, if the user tries to save an existing value for a unique field, I want to be able to throw an error for that field specifically, which should show up in $errors.
Is there currently a way to do that?
Thank you.

There is a way, hidden deep in the documentation (it needs some cleanup for sure):
import { setError } from 'sveltekit-superforms/server';

return setError(form, 'field', 'Not unique!')

It returns a fail(400, { form }) so it can be returned immediately, or more errors can be added by calling it multiple times before returning.

Currently documented here: https://github.com/ciscoheat/sveltekit-superforms#server

Thank you!

@ciscoheat This is still documented but it's wrong. There is currently no field option for setError.
It is (form, error, options). I don't know how to set a field error any more.


Implement cache client event listeners

Is there an existing issue that is already proposing this?

[X] I have searched the existing issues

Is your feature request related to a problem? Please describe it
I often use console commands for long running jobs such as data migrations. I also have some workers that run in a headless Nest application.
In both instances, I occasionally have issues where the Redis cache client throws an ETIMEDOUT error. This has the effect of crashing the entire program, and no amount of try/catching seems to catch the error.
I believe the correct way to deal with this is to set up an event listener on the redis client itself, however I have not been able to find a way to get the client once it's been instantiated.
Unless I'm missing something, there is no opportunity to set up an error listener in the following example code:
    CacheModule.registerAsync<RedisClientOptions>({
      useFactory: async () => {
        const config = await getRedisConfig();

        return {
          ...config,
          store: redisStore,
        };
      },
    }),

Describe the solution you'd like
Being able to grab the client and set up listeners somewhere before it is used seems to be important. Ideally one could set up listeners at various touchpoints:

in the configuration itself
by exposing the client (or an .on() method) at runtime

Event listeners may be specific to the Redis client, in which case this would be a better request for that library.
Teachability, documentation, adoption, migration strategy
A possible API
Setting up listeners at config time:
    CacheModule.registerAsync<RedisClientOptions>({
      useFactory: async () => {
        const config = await getRedisConfig();

        return {
          ...config,
          store: redisStore,
          on: {
            error: globalCacheErrorHandler,
            // other event listeners
          }
        };
      },
    }),

Setting up listeners at run time
 @Injectable()
export class CacheService {
  constructor(@Inject(CACHE_MANAGER) private cacheManager: Cache) {
    this.cacheManager.on('error', this.handleCacheError);
  }

  private handleCacheError(err: Error) {
    // ....
  }
}

What is the motivation / use case for changing the behavior?
As it stands, it's impossible to react to events emitted by the underlying client.

anything new on this issue?

For listening to the events from RedisClient, once the module has been initialized, I could write a listener for it as follows (I am using cache-manager-redis-store):
export class RedisClientEventListener implements OnModuleInit, OnModuleDestroy {
  private readonly logger = new Logger(RedisClientEventListener.name);
  private readonly redisStore: RedisStore;

  constructor(@Inject(CACHE_MANAGER) readonly cacheManager: Cache) {
    this.redisStore = cacheManager.store as unknown as RedisStore;
  }

  onModuleInit(): void {
    const client = this.redisStore.getClient();

    if (!client) {
      this.logger.error('no redis client initialised');
    }

    if (client.isReady) {
      this.logger.log('redis client is connected and ready');
    }

    client.on('connect', () => {
      this.logger.log('redis client is connecting to server...');
    });

    client.on('ready', () => {
      this.logger.log('redis client is ready');
    });

    client.on('end', () => {
      this.logger.log('redis client connection closed');
    });

    client.on('error', (error: Error) => {
      this.logger.error(error, 'redis client received an error');
    });
  }

  async onModuleDestroy(): Promise<void> {
    await this.redisStore.getClient().quit();
  }
}

and then the module that imports and initializes CacheModule extends the above:
@Module({
  imports: [
    CacheModule.registerAsync(cacheModuleOptions),
  ],
  controllers: [SomeController],
})
export class SomeModule extends RedisClientEventListener {}

This works well to listen to the errors and handle those and the app doesn't crash, however, if the app is unable to connect to the client during initialisation itself, these listeners don't get a change to get registered and the app crashes. So far even I haven't found a way to avoid the app from crashing when app is unable to connect while initialising CacheModule.

I am stuck on the exactly the same thing :(

There's not much we can control here, as we're relying on the behavior of the cache-manager. However, I was able to put together an example that demonstrates how you could listen for initialization errors, and, if Redis is unavailable, fall back to an in-memory store:
CacheModule.registerAsync({
  useFactory: async () => {
    try {
      const store = await redisStore({
        socket: {
          host: 'localhost',
          port: 6380,
        },
      });

      // Listen for Redis connection events
      store.client.on('connect', () => {
        console.log('Connected to Redis');
      });

      store.client.on('error', err => {
        console.log('Redis error:', err);
      });

      // If we reach this point, Redis is available
      return [
        {
          store: 'memory',
          max: 100,
          ttl: 50,
        },
        {
          store: store as unknown as CacheStore,
          ttl: 3 * 60000, // 3 minutes (milliseconds)
        },
      ];
    } catch (err) {
      console.error('Error initializing Redis:', err);

      // Fallback to in-memory store
      return [
        {
          store: 'memory',
          max: 100,
          ttl: 50,
        },
      ];
    }
  },
}),

In the next major release, we're going to upgrade to v6 (ref https://github.com/nestjs/cache-manager/pull/508) - this should make things a bit easier moving forward


adding feature of user search

Description

Adding three functions for user search by id/name/username using mongoose.
Related Issue

Solves #20

Type of change

[x] New feature (enhancement)

How Has This Been Tested?

This schema has not been tested.
Checklist:

[x] Does a similar (open or closed) pull request not already exist?
[x] Is the pull request head repository a fork repository?
[x] Is the pull request compare branch a development branch?
[x] Is the code documented, particularly in hard-to-understand areas?
[ ]  Does the code build without new warnings?
[ ] Has testing been performed that proves changes are effective and work?
[ ] Has a self- and/or peer-review of the code been performed?
[x] Have dependent changes been merged and published in downstream modules?
[ ] Does all new and existing automated testing pass?
[x] Is the person responsible for the repository assigned to the pull request?
[x] Is the pull request linked to a project?
[x] Is the pull request linked to a milestone?

For more information, refer to the Contributing Guidelines document.

I will help fix this soon

I have left some suggestions.
It is not a good idea to rely on data being manually entered in the database for the tests to pass.

what i was thinking is that I manually inserted some data into the databbase and try if i can search those data by id,  and I also try to search some data which does not exist. So what else could I do in order to test the function?

I have left some suggestions.
It is not a good idea to rely on data being manually entered in the database for the tests to pass.

what i was thinking is that I manually inserted some data into the databbase and try if i can search those data by id, and I also try to search some data which does not exist. So what else could I do in order to test the function?

The problem with that is new changes have all collection data in the test database wiped before each test and only the test database is used for tests.


Extension do not work properly on Brave

Hello, I would like to raise an issue.
When I open postwoman.io with the extension installed, it cannot complete the request, just loading.

On the console, it says something like this:

Browser version:
Version 1.10.97 Chromium: 83.0.4103.116 (Official Build) (64-bit)

Hi there, sorry about that, there was a small bug with the published version right now, I have uploaded a version with the bug fixed but Chrome Web Store hasn't finished reviewing it.

Oh, okay then


named_args: raise or print error if both or either formula/cask unreadable

[x] Have you followed the guidelines in our Contributing document?
[x] Have you checked to ensure there aren't other open Pull Requests for the same change?
[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[x] Have you written new tests for your changes? Here's an example.

[x] Have you successfully run brew typecheck with your changes locally?
[x] Have you successfully run brew tests with your changes locally?
[x] Have you successfully run brew man locally and committed any changes?

Related: https://github.com/Homebrew/brew/pull/10405, https://github.com/Homebrew/brew/pull/10422
This PR changes how some edge cases are handled in named_args.rb:

When both the formula and the cask are unreadable, raise the FormulaUnreadableError
When both the formula and the cask are present and one of them is unreadable, print the error and a warning that the readable one will be used.

brew info output when both the formula and the cask are unreadable
Before:
$ brew info crunch
Error: No available formula or cask with the name "crunch".

After:
$ brew info crunch
Error: crunch: /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/crunch.rb:5: syntax error, unexpected tSTRING_BEG, expecting end
  sha 256 "6a8f6c3c7410cc1930e6854d1dadc...
          ^

brew info output when only the formula is unreadable
Before:
$ brew info crunch
crunch: 4.0.0

After:
$ brew info crunch
Error: Failed to load formula: crunch
crunch: /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/crunch.rb:5: syntax error, unexpected tSTRING_BEG, expecting end
  sha 256 "6a8f6c3c7410cc1930e6854d1dadc...
          ^
Warning: Treating crunch as a cask.
crunch: 4.0.0
...

brew info output when only the cask is unreadable
Before:
$ brew info crunch
crunch: stable 3.6 (bottled)
...

After:
$ brew info crunch
Error: Failed to load cask: crunch
Cask 'crunch' is unreadable: /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Casks/crunch.rb:3: syntax error, unexpected tSTRING_BEG, expecting end
  sha 256 "6969fcb91e5a93b9d9e604cca2e6a...
          ^
/usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Casks/crunch.rb:17: syntax error, unexpected end, expecting end-of-input
Warning: Treating crunch as a formula.
crunch: stable 3.6 (bottled)
...

Review period will end on 2021-01-28 at 18:38:24 UTC.

Review period will end on 2021-01-28 at 18:38:24 UTC.

Review period ended.

Review period ended.


Avoid double imports

A little refactor to enables the [rule.duplicated-imports] which seems to be able to pick-up these cases. And fix existing cases whereby a file imported a package twice.

Codecov Report

Merging #17569 (aa72c60) into main (bd613c7) will decrease coverage by 0.00%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main   #17569      +/-   ##
==========================================
- Coverage   45.49%   45.48%   -0.01%
==========================================
  Files         793      793
  Lines       88811    88811
==========================================
- Hits        40401    40397       -4
  Misses      41897    41897
- Partials     6513     6517       +4

Impacted Files
Coverage Δ

models/issue.go
58.17% <0.00%> (ø)

routers/api/v1/repo/transfer.go
60.46% <0.00%> (ø)

modules/migrations/gitea_uploader.go
6.62% <50.00%> (ø)

modules/convert/convert.go
79.66% <100.00%> (ø)

modules/git/repo_base_nogogit.go
82.85% <0.00%> (-2.86%)
:arrow_down:

modules/git/utils.go
68.05% <0.00%> (-2.78%)
:arrow_down:

models/issue_comment.go
51.31% <0.00%> (-0.59%)
:arrow_down:

modules/queue/workerpool.go
48.85% <0.00%> (+0.76%)
:arrow_up:

modules/queue/queue_channel.go
96.66% <0.00%> (+1.66%)
:arrow_up:

Continue to review full report at Codecov.

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


Improve docs

Two real quick, really minor improvements to the documentation.

Waycorner is in Nixpkgs now (and has been for a while, I packaged it last year) so we can tell the users how to get it if they are using Nix
Fixes a minor issue with the example config, where color = #FFFF0000 would cause an error.

I'd love to be able to include it in the automated deployment that is setup for AUR and Cargo. Though considering that is not the case as of right now, I want to distance myself and the project from any potential issues by calling out that it's unrelated.

Nixpkgs is entirely independent from any kind of CI infrastructure - I've configured the package to

build from the latest available tag
automatically update whenever there is a new tag in this repository

As long as you make a new release, the package will follow the source from this repository. It is, for now, not possible to push to nixpkgs from this CI but it could be possible to make a standalone Nix package in the repository here, should you wish to.
I'll apply other changes when I'm back on my desktop.

Thanks for the context! I've updated the suggestion a bit. I did not know how nixpkgs worked. Slightly annoying that it cannot be updated automatically. Oh well. 🤷♂ Just a mention stating it might not be up-to-date is fine.

In theory, it can be automated. There exists an unattended updates bot that will periodically open pull requests for packages that have new release tags. If this is undesirable, I can change the package to follow the latest commit - but that is discouraged in Nixpkgs convention.
We can simply add a reminder that people may open an update request if the package is updated - I, or some other available maintainer may get on it.

@NotAShelf it's all up to you. If you want automation, go for it. If you don't, all good too.
I'm primarily saying that as long as there is no automation, there should simply be a small mention of it possibly being slightly out of date because it needs manual updating.

I've updated the Nix section to apply your suggestions, and hopefully clear any doubts for the less technically inclined.
I'll make a pull request to add an update script for the Nixpkgs derivation tomorrow (my open PR to renovate the package got merged before I could do it today).

Thanks again for maintaining the Nix package and the PR. ❤


integrate cargo-update

Describe the problem you are trying to solve
cargo does not have an easy way to see what bins can or should be updated. using https://github.com/nabijaczleweli/cargo-update is required for this
Describe the solution you'd like
cargo-update behavior built into cargo official in some way
Notes

the --force option does update stuff, but only if you know it needs to update. no easy way to check this other than hope or looking up each bin on lib.rs or crates.io --force is also one at a time vs all like cargo-update

Thanks for the report. Going to close this in favor of #9527. I think the approach is more or less the same — integrate some parts of https://crates.io/crates/cargo-update into cargo.


Add ResourceAssociation update operation

Description of changes:
This change allows the ram controller to associate
resourceArns, principals, and sources to a resource share
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

also @michaelhtm PR description please :)

/retest

/retest


I/O weirdness

Bug Summary
In the latest release, the I/O for script execution seems to be off. I'm having to hit enter often in order to push through progress of an executable. There also appears to be a lot of extra whitespace thrown in. This is especially noticeable when working with serial and parallel executables.
Additional context
No response

I suspect that this may have something to do with the latest log mode changes


Add IL scanner phase

Sending this out to give people a chance to give early feedback.
This adds an initial (empty) implementation of an IL scanner that will scan method bodies, starting from the provided compilation roots, and expand the dependency graph based on scanning the IL, to determine the set of methods (and types, and other interesting artifacts) that will be compiled.
This nicely plugs into the existing architecture where the scanner looks almost just like another codegen backend - we have a separate NodeFactory for it, a separate builder class, and a separate compilation class. The actual IL parsing reuses the importer written for CppCodegen and ILVerifier.
The results of the scanning phase will flow into the metadata generator and potentially other parts of the system.

Dropping the WIP tag. I still see a couple diffs when comparing to RyuJIT, but at least some of them are caused by #3608 that I'll need to go fix. Let's review this.
@davidwrighton @nattress @jkotas?

@kouvel @AntonLapounov @sergiy-k The Ubuntu Debug assertion might be unrelated. There's very little code in this pull request that actually runs (pretty much only the small refactorings to RyuJit*.cs and CppCodegen*.cs are live code). I don't want to ask dotnet-bot to retests this just yet in case there's some artifacts that could be salvaged.
Running test src/Simple/BasicThreading BasicThreading
FinalizeTest passed
Unhandled Exception: System.Diagnostics.Debug+DebugAssertException:

   at BasicThreading!<BaseAddress>+0x549e86
   at BasicThreading!<BaseAddress>+0x540dd0
   at BasicThreading!<BaseAddress>+0x533752
   at System.Threading.WaitSubsystem.WaitableObject.get_IsSignaled()
   at System.Threading.WaitSubsystem.WaitableObject.SignalAutoResetEvent()
   at System.Threading.WaitSubsystem.WaitableObject.SignalEvent()
   at BasicThreading!<BaseAddress>+0x5b5fab
   at System.Threading.EventWaitHandle.SetCore(IntPtr handle)
   at System.Threading.EventWaitHandle.Set()
   at System.Threading.Lock.ReleaseContended()
   at System.Threading.Lock.ReleaseCore()
   at System.Threading.Lock.Release()
   at BasicThreading!<BaseAddress>+0x54676b
   at BasicThreading!<BaseAddress>+0x555d28
   at BasicThreading!<BaseAddress>+0x57972f
   at BasicThreading!<BaseAddress>+0x55da4d
   at BasicThreading!<BaseAddress>+0x550c74
   at ThreadStaticsTestWithTasks.<>c.<Run>b__2_0(Object param)
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()
   at System.Threading.Tasks.Task.ExecutionContextCallback(Object obj)
   at BasicThreading!<BaseAddress>+0x722e3e
   at System.Threading.ExecutionContext.Run(Threading.ExecutionContext executionContext, Threading.ContextCallback callback, Object state)
   at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Threading.Tasks.Task& currentTaskSlot)
   at System.Threading.Tasks.Task.ExecuteEntry(Boolean bPreventDoubleExecution)
   at System.Threading.Tasks.Task.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at BasicThreading!<BaseAddress>+0x74f32d
src/Simple/BasicThreading/BasicThreading.sh: line 2: 26030 Aborted                 (core dumped) $1/$2
fail

Looks good. I dislike that we can only have one variant of the ilscanner per binary due to the partial class implementation sharing, and we may wish to have more implementations of this thing in the future, but that's not an issue which I'd like to see you fix at this time.  (Its fixable with no/negligible perf loss by going enthusiastic with generics and constraints, but I'm not sure I want to propose that.)

I dislike that we can only have one variant of the ilscanner per binary due to the partial class implementation sharing

Yes, I spent a couple hours trying to find a scheme where we could share the importer differently, but ended up kicking CppCodegen out of this assembly instead (#3468) - because as you noted, we had a need to have the importer in both, and I didn't want us to pay the perf penalty.
We might be able to come up with a scheme where we make the scanner itself shareable (because most parts of the scanner-specific importer are boring).

The Ubuntu Debug assertion might be unrelated. There's very little code in this pull request that actually runs (pretty much only the small refactorings to RyuJit*.cs and CppCodegen*.cs are live code). I don't want to ask dotnet-bot to retests this just yet in case there's some artifacts that could be salvaged.

I don't see a link to a dump in the build output, and I couldn't find a dump on dumpling corresponding to this failure based on the build time. There's probably not much else in the build output that would be useful. Would need to try to repro this or figure it out from the stack trace.


move get_call_graph to analysis, resolves #472

Codecov Report

Merging #477 into master will increase coverage by 0.05%.
The diff coverage is 8.33%.

@@            Coverage Diff             @@
##           master     #477      +/-   ##
==========================================
+ Coverage   69.93%   69.98%   +0.05%
==========================================
  Files          43       44       +1
  Lines       13795    13915     +120
==========================================
+ Hits         9648     9739      +91
- Misses       4147     4176      +29

Impacted Files
Coverage Δ

androguard/core/analysis/analysis.py
72.65% <8.33%> (-2.56%)
:arrow_down:

androguard/core/__init__.py
92.7% <0%> (ø)

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 321e601...5bcf64b. Read the comment docs.


Feature wtmp

WIP utmp/wtmp

should work under linux
on freebsd wtmp is ok, but updwtmpx() isn't known

Please resolve conflict.

I'll hand merge this into wtmp branch.

@moobyfr I've hand merged and pushed to wtmp branch. If you will make further changes, please checkout wtmp branch and make changes based on the branch.


tagged unions support added.

tagged unions support added. See https://docs.pydantic.dev/2.0/usage/types/unions/#discriminated-unions-aka-tagged-unions

Codecov Report

Merging #79 (667e3d2) into dev (a6d40e6) will decrease coverage by 0.84%.
The diff coverage is 93.53%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
@@            Coverage Diff             @@
##              dev      #79      +/-   ##
==========================================
- Coverage   93.11%   92.27%   -0.84%
==========================================
  Files          23       23
  Lines        1089     1204     +115
==========================================
+ Hits         1014     1111      +97
- Misses         75       93      +18

Flag
Coverage Δ

unittests
92.27% <93.53%> (-0.84%)
:arrow_down:

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

Impacted Files
Coverage Δ

pydantic_xml/element/native/lxml.py
0.00% <ø> (ø)

...ydantic_xml/serializers/factories/typed_mapping.py
73.91% <73.91%> (ø)

pydantic_xml/serializers/factories/tagged_union.py
86.00% <86.00%> (ø)

pydantic_xml/serializers/factories/union.py
87.95% <87.17%> (+0.14%)
:arrow_up:

pydantic_xml/serializers/factories/mapping.py
88.75% <89.33%> (-1.88%)
:arrow_down:

...ydantic_xml/serializers/factories/heterogeneous.py
89.58% <90.90%> (-1.49%)
:arrow_down:

pydantic_xml/serializers/factories/homogeneous.py
92.00% <91.48%> (+0.62%)
:arrow_up:

pydantic_xml/serializers/factories/primitive.py
95.55% <95.29%> (+1.52%)
:arrow_up:

pydantic_xml/serializers/factories/model.py
95.48% <95.88%> (+0.19%)
:arrow_up:

pydantic_xml/element/element.py
99.42% <96.55%> (+0.03%)
:arrow_up:

... and 8 more


Turtle enhancements

By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.

This PR provides enhancements to the turtle visualizer:

Alongside the "domain" input there's a modulus and the sequence is automatically taken with respect to that modulus (just like chaos does), so you shouldn't have so many boring thumbnails that go two steps and die.
The number of elements in the "angles" list should match the domain size; causes warning. Similar for other lists.
The step size is now a list of step sizes, like the angles, so these can be set individually.
There's a first term option, so you can start later in the sequence; warns if out of range.
There's a path length option, warns if goes beyond the max number of terms if using OEIS.
The default behaviour is to draw the full path at a rate of 1 term per frame.
If you want it to draw the full path instantly, you should just set the rate to the same as pathlength (I hope this is clear enough to discover?)
If the sequence is a formula, then potentially there are infinitely many terms; setting the path length to zero will keep drawing forever or until terms run out
If you use the "Folding animation" list, the angles in the angle list will gradually change, causing the path to "fold" gradually over itself like a protein. This is fun for finding other variations on a cool picture you've found. This is also an animation feature. You can combine it with growth, or use just one of them.  If you close this checkbox, it hides the folding list and turns off folding.
Gallery items have been preserved and added.

Note: This turtle visualizer is not backward compatible to past URLs.
Things to consider implementing in a subsequent PR:

more colour schemes
automatic zooming and panning to keep the path in view
manual zooming and panning by user

As I find some more nice examples, posting a few links here:
http://localhost:5173/?name=Uncoiling+Dragon+2&viz=Turtle&modulus=2&domain=0+1&turns=20+1690.04&steps=20+100&growth=10&startEndControl=true&firstTerm=1&pathLength=10000&foldAnimation=true&folding=30+0&bgColor=37674c&strokeColor=b5cfb4&seq=OEIS+A002260

I have changed my opinions on my parameter handling methods.  I'm converting to draft.

Things we still need to fix that have to do with parameter handling:

Right now if you have a sequence and choose the visualizer Turtle, and the Turtle has a warning because the default first term 0 is not valid, it won't just warn and draw.
Same situation, it won't populate the max/min values for things like number of terms or first term until you change those fields.  Same for updating them (because they are dependent on other field values).

Right now turtle just keeps drawing and drawing more of the path by default, but it will eventually get laggy if you don't pause it.  Should we do something about that?

Sorry for the git mess, I rebased onto ui2 properly this time but I probably didn't do it optimally.

After the rebase, a change in behaviour:  when you change one of the NUMBER_ARRAY parameters (domain, turns, etc) it restarts the animation but doesn't take into account the update.  however, if you hit the restart button on the header, it will restrart WITH taking into account the update.

After the rebase, a change in behaviour: when you change one of the NUMBER_ARRAY parameters (domain, turns, etc) it restarts the animation but doesn't take into account the update. however, if you hit the restart button on the header, it will restrart WITH taking into account the update.

I suspect somehow that the issue here is that we are checking if a parameter changed, but arrays values are by reference, so maybe the reference hasn't changed?

eventually get laggy

Do you mean that the time to draw each individual additional stroke increases? That seems strange --can you post a URL for which that happens?

eventually get laggy

Do you mean that the time to draw each individual additional stroke increases? That seems strange --can you post a URL for which that happens?

This only applies when we're doing the folding animation, which requires redrawing the path each frame.  Take for example a random walk with folding turned on: http://localhost:5173/?name=Random&viz=Turtle&steps=1+1+1+1+1&walkAnimation=true&growth=100&startEndControl=true&foldAnimation=true&folding=100&seq=Random

I'm seeing really weird behaviour on this example: http://localhost:5173/?name=Beatty+forever&viz=Turtle&modulus=3&domain=0+1+2&turns=79+0+45&steps=1+1+1&walkAnimation=true&growth=100&startEndControl=true&start=0+200&folding=0+10+0&bgColor=e9eee3&strokeColor=4b7a81&seq=Formula&formula=floor(n*sqrt(2)) ... if I use the browser reload button vs. the refresh button on the header, I see a different picture... but reliably repeatable.  I get one picture when reloading browser and a different when using refresh button.  Repeatable with different browsers, and after restarting numberscope.  Thoughts?

But maybe you could rebase this code in the meantime on ui2? We could even cross our fingers and hope it improves the situation... Let me know when it is synced up with ui2.

Just an update, in case anyone is worrying about what's going on here.  I rebased and force pushed, but actually the tests don't pass.  Then I worked on fixing the merge issues and got to the point the code will run, and committed despite tests not fully passing yet.  I know the idea is only to commit things that work, but the rebase broke lots of things.    So I'll keep going until I get back in a state where all tests pass.

Of course that's OK under the circumstances. And I also want to make sure you know that if the behavior of the Turtle visualizer has legitimately changed in such a way that the expected output of one of the previously existing tests is now different, you can get the tests to pass by changing the expected snapshot to match the new behavior. There is even a command-line flag for doing this snapshot copying; it's --update-snapshots I believe. Only use it when the only tests that are failing are ones where the desired result of the test has changed, as it will update all snapshots.

PS. just wanted to point out that in your latest additions to math.ts, the new functions say in their comments that  they return number but they seem in fact to return ExtendedBigint.

The most recent push has us back to a functioning turtle after rebase in the most important respects, but there are still bugs and issues and the e2e tests are failing.  I'm not confident enough in it to take new snapshots for the tests yet.

One question:  I was doing a try...except to catch CachingError before but now lint objects to this -- are we not throwing CachingErrors anywhere anymore, or did I mix up when changing the imports?   Not sure why that's suddenly showing up.

Second question:   The P5GL panning is working (yay!) but the wheel zoom is not.  I need to run some turtle code (to clear canvas) upon wheel events, but the WheelEvent() function that I'm inheriting complains it wants an event input.  We had to add mouseWheel() to P5Visualizer.ts at one point, but of course P5GL is paying attention to these events and maybe something is getting interrupted.  Not sure if you have a suggestion here, or if I'm using the P5GL controls wrong.

Another question related to debugging.  I'm seeing weird behaviour when I change the number of terms in the sequence pane.  Which stuff gets re-run when that changes?

One question: I was doing a try...except to catch CachingError before but now lint objects to this -- are we not throwing CachingErrors anywhere anymore, or did I mix up when changing the imports? Not sure why that's suddenly showing up.

It's the only occurrence of the identifier CachingError in the file, so it must perforce be undefined. You need to import CachingError from @/sequences/Cached.ts. As to why there was no complaint before, I cannot say.

the wheel zoom is not.

OK, I just pushed a commit to your branch that improves this situation a little bit. Make sure you pull it and incorporate it into your working tree (you may need to stash your changes before the pull). But it does not fix it altogether.  To see that wheel zoom is doing something, go to a drawing turtle where you can see what's going on, and roll your mouse wheel rapidly a moderate distance and then left-drag (pan) the view immediately afterward. You should see the whole drawing zoom in or out. But if you just mouse wheel gently, the event seems to be getting lost; and if you don't pan, it seems as though what's already been drawn doesn't get refreshed.
I am actually not sure what is going on here. A single frame of Turtle is super quick, there's almost nothing to do. So maybe this is some kind of race condition between the event handling and the draw()? But if so, why does panning work so nicely (and fix the problem with zooming)? And why does zoom work just fine in Histogram?
Do you want to poke around about this in p5 webgl posts online, or do you want me to debug it when I next have time? Whichever you would enjoy more ;-)
Maybe @Vectornaut has some ideas?

when I change the number of terms in the sequence pane. Which stuff gets re-run when that changes?

As far as the visualizer knows, it's a completely new sequence. So everything that gets run on a reload should get re-run. Is there something that seems not to be getting run that you want it to run? Or conversely, is something being re-run that you don't think should be?

Thanks for the responses, that fixed some stuff!  I'll keep mucking about.

I will also say, that since you're now drawing with WebGL, it's actually drawing in 3D, so there's no practical reason not to have a 3D turtle and re-enable the "orbit" control as well. But maybe you'd feel that 3D Turtle should be a separate visualizer? I am kind of thinking that we should put it in either as an extra panel you can pop up in this one or as another closely related visualizer that shares code. I feel like it would be good to go with because I am sure there are some sequences and recipes that will look very good in 3D, and I think it will have "wow factor" when we do release...

There's a Wow factor to just reading this comment!  But first let's get the 2d working reliably!

Yes, I do agree, first things first. And 3D turtle graphics is noticeably more intricated than 2D: In 2D, just forward and turn suffice, but in 3D, you need at least forward, yaw (that's the equivalent of 2D turn), and either pitch or roll; it's probably easiest just to allow either/both pitch and roll.
Here's hoping that some nice sequences with relatively simple 3D turtle instructions produce knotted paths ;-)

Ok, with this newest push, I think Turtle is basically working properly.  But it still has this very weird behaviour where a hard refresh (browser reload) is reliable and a soft refresh (change parameter) can result in bad behaviour.  When looking at the console, I'm seeing that bad behaviour happens with the array I'm storing the path in is a "proxy" (something to do with vue's responsive whatever stuff).  That's my intuition.  Anyway, feel free to use this version to debug this issue -- you can reproduce it as follows:

load up http://localhost:5173/?name=Wait+For+It&viz=Turtle&domain=-1+1&turns=30+120&steps=30+30&speed=10&strokeWeight=2&bgColor=5d509f&strokeColor=7a9f6f&seq=Formula&formula=sign(sin(n%2B1))&last=199&length=200
open the console, where you can get some debug info
change the number of terms in the sequence to 700; the image won't change
hard refresh -- then it will show all 700 steps
more of this type of thing, changing number of terms up and down produces unpredictable behaviour like range errors in the console; but a hard refresh always fixes it

Ok, I just pushed a couple of commits that help with all of the issues you noticed; you'll want to sync up with it. Unfortunately there are several changes, so I will try to enumerate them all in minute detail so you can see what happened. One thing I didn't change (I don't think I did, anyway) is the collection of console.logs. You'll of course have to remove those before the PR is really ready for review. Anyhow:

I changed turtleData -> TurtleData because I have a really hard time with types that don't start with a capital letter. It's a pretty strong convention out there in the wild.

Indeed, the Vue reactivity was invading your datastructures. Basically, it attempts to insert itself recursively into all objects and subobjects and subsubobjects of anything that appears as a parameter of any component, and since the visualizer is a parameter of the canvas, Visualizer objects are targets of this. So I made the TurtleData class always mark itself as raw (i.e., opt out of reactivity) since you never use TurtleData as a "web control" where you need changes the user makes to tie into your data nor changes the code makes to auto-update the page. And similarly I always mark the path as raw. The reason this has not been affecting most visualizers is that it only affects object (and array) data, not numbers, strings, etc. And most visualizers only have these sorts of basic datatype fields.

There were a number of quantities (like firstIndex, which comes from Sequence.first) that are always finite. You were using ExtendedBigint for some of these quantities. I switched them to bigint. Basically ExtendedBigint typically makes code more complicated, so should only be used for quantities that truly can be infinite. These changes led to all instances of bigadd and bigsubtract going away, so I recommend removing those from math.ts (i.e., don't add unused code).

Now we come to the core of what was going on: you were doing some of your setup() in your checkParameters() method. The conceptual difficulty is that checkParameters() is called with a tentative collection of parameters, and just wants to know "are these parameters valid?" It is not promising that these _will be_the parameters of the visualizer, it just wants to know if they would work. So you should never save values from the input params object into the "state" of your visualizer, as was happening with growth for example. Pretty much anything that looks like this.blah = some_code_with(params.anything) is a red flag in a checkParameters() function.
The further practical difficulty is that this is checkParameters() for the visualizer, but the sequence length is now a parameter of the sequence. Hence, when the sequence length changes, no parameters of the visualizer have changed, so there is no parameter checking for the visualizer to do, so checkParameters of the visualizer is not called, and so this.pathLength was stuck with the old, now wrong, value, causing all kinds of weird behavior.
The solution was simple: I just moved these steps of initializing some of the data of the visualizer into setup() rather than checkParameters(). The values that were being set in checkParameters weren't being used before setup() anyway, so there was no need to try to rush to set them.
I also just made it an error (that the visitor has to fix) to try to view an infinite sequence in a single frame. That way there is no need to change the parameters that are set in the interface -- the visitor will do it.
A couple final comments on this one: I don't see that growthInitial is really being used; it's just being set, and immediately growth is set from it, and then it's never referred to again. I think it can be eliminated so that you'll just have growth.

You had a presketch that was just calling super and then refreshParams. But as no params were changing in this function, there was no need to call refreshParams, and so that meant that the function wasn't really doing anything except call super, and there's no need to implement a function just to call super. So I removed the presketch() method altogether.

OK, I think that's everything. If there's anything confusing about the changes I made, don't hesitate to ask by whatever means is most convenient. Hopefully this will get you on track to complete the Turtle.

Thank you so much!  That's a huge help!  I have learned several things from this, and the code is simpler now.

Thank you so much! That's a huge help! I have learned several things from this, and the code is simpler now.

My pleasure. Quick comment re your latest commit: you don't need to both (a) force speed to be between 1 and 100 using the validate() function, and (b) set growth to the min of speed and 100. Option (a) will make (b) moot, and (b) means it's not really important whether the visitor set speed over 100 (as long as there is feedback that it's not having any additional effect past 100). Choose whichever you think makes the nicer interface.

Thanks for keeping an eye as I work on this!  I'm still having some concerns over the visualizer getting laggy, even with the 100 step limit.  For example, when folding, it will eventually lag up, because every frame is new and later frames have huge numbers of terms.  My fix for P5GL at the moment is also to redraw (like when folding) if the user zooms or pans.  This may not be the desired way to handle that.  So there are two things I'm currently pondering: (1) is there a way to avoid the redraw and have zoom/pan work?; and (2) What's the best philosophy on lagginess?  What's better -- to force stop drawing to avoid lagginess or let the user suffer the consequences of letting an infinite sequence just keep running and running in folding mode?  There are many unsatisfying things about the way folding is handled at the moment, so it might even make sense to refactor in some ways... but even with the best possible code, it will eventually lag on a really long path.

My fix for P5GL at the moment is also to redraw (like when folding) if the user zooms or pans. This may not be the desired way to handle that

My experience with Histogram was that WebGL mode simply remembered (internally to p5.js) everything that had been drawn, and so zoom/pan Just Worked™ with no further effort on my part. In other words, ideally you should not need to re-draw (unless what you want to show has changed) to get zoom/pan working. If that's not the case, then one or both of us should put the code back in a state where it is not trying to re-draw just because of a zoom or pan and then debug what's going on.

when folding, it will eventually lag up, because every frame is new and later frames have huge numbers of terms.

Hmmm. Obviously the issue here is that changing a turn angle at one vertex changes the coordinates of all following vertices. So you're pretty much stuck redrawing. I pondered this a bit. The ideal would be if you could make the redrawing itself incremental over multiple frames, to keep the frame rate up. But that bit about all later vertices moving makes that tough.
The most likely-to-give-best-speed approach I could think of after pondering a bit would be to use a custom visualizer for turtle that just directly inserts an svg element into the canvas (rather than basing on P5Visualizer, or P5GLVisualizer, at all). Then the main turtle track itself could be just a single svg <path> element; you would literally draw additional steps by appending to the specification of this path. The benefit here is that <path> has a "relative step" directive, so on a given frame when you were folding, you could modify some fixed maximum number of these directives and then let the browser's svg engine (which is likely faster than anything we could write in JavaScript) re-render the path. That way you could indeed incrementally fold along the path, spread over multiple frames.
But that's a lot of work, and presumably out of scope for the current overhaul. So short of that, here are a couple of ideas, both based on picking a true maximum number of segments you're willing to draw per frame (presumably significantly larger than the "speed" limit). I'll call that maxGrowth.

When folding, once the path length has become greater than maxGrowth, switch to redrawing the path after a fold-angle increment with growth=maxGrowth (and not incrementing the fold angles again until enough frames have gone by to get back out to the current "growth tip" at the visitor's speed setting).
When folding, put a hard limit on the number of terms to draw at maxGrowth.

What's the best philosophy on lagginess? What's better -- to force stop drawing to avoid lagginess or let the user suffer the consequences of letting an infinite sequence just keep running and running in folding mode?

I do think people's patience differs. So I do think that some softness to limits is good, so that someone exploring can push to the limits of what they are willing to put up with. On the other hand, I think there do need to be other measures/hard limits that prevent the rendering from getting to the point where the browser starts to just be unusable/the Numberscope code seems buggy or unprofessional. I know that's a wishy-washy answer, sorry. Maybe the best compromise is some sort of self-monitoring and posting warnings as the path length and amount of stuff that needs to be drawn and re-drawn every frame gets into dangerous territory.  You could watch that timing and suggest to the visitor that they throttle things back and/or automatically throttle things back when/if there's evidence of lagginess.

different, you can get the tests to pass by changing the expected snapshot to match the new behavior. There is even a command-line flag for doing this snapshot copying; it's --update-snapshots I believe. Only use it when the only tests that are failing are ones where the desired result of the test has changed, as it will update all snapshots.

Could you elaborate on this?  I tried the following without success:
katestange@ducked-tape:~/data/code/numberscope/frontscope/src/shared$ npx playwright test --update-snapshots
TypeError: Cannot redefine property: Symbol($$jest-matchers-object)

    at /home/katestange/data/code/numberscope/frontscope/node_modules/@vitest/expect/dist/index.js:21:10
TypeError: Cannot redefine property: Symbol($$jest-matchers-object)

    at /home/katestange/data/code/numberscope/frontscope/node_modules/@vitest/expect/dist/index.js:21:10
TypeError: Cannot redefine property: Symbol($$jest-matchers-object)

    at /home/katestange/data/code/numberscope/frontscope/node_modules/@vitest/expect/dist/index.js:21:10
TypeError: Cannot redefine property: Symbol($$jest-matchers-object)

    at /home/katestange/data/code/numberscope/frontscope/node_modules/@vitest/expect/dist/index.js:21:10
SyntaxError: /home/katestange/data/code/numberscope/frontscope/src/sequences/Cached.ts: TypeScript 'declare' fields must first be transformed by @babel/plugin-transform-typescript.
If you have already enabled that plugin (or '@babel/preset-typescript'), make sure that it runs before any plugin related to additional class features:
 - @babel/plugin-transform-class-properties
 - @babel/plugin-transform-private-methods
 - @babel/plugin-proposal-decorators
  110 |     {
  111 |         name = 'An uninitialized generic sequence'
> 112 |         declare params: typeof fullDesc // Since Paramable is no longer ...
      |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  113 |         // ... generic, we need to tell TypeScript what the (type of the)
  114 |         // params for this instantiation are.
  115 |
Error: No tests found

I'm a bit wary of trying too many things without understanding what's happening.

Could you elaborate on this?

I believe the command should be npm run test:e2e -- --update-snapshots. Try that and see how it goes.

I see in the latest commit that you suggest removal of Lattice Work from the featured gallery. Well, we have a "curate featured gallery" issue on the list for alpha (#428), and I think @Vectornaut and I were just assuming you would be the curator, so as far as I am concerned just go ahead and remove it if you like (and add another Turtle one that shows off some of the new features if you have a good one). Cheers.
P.S. I am assuming that the difference in behavior between soft and hard resets that was troubling you before the end-to-end testing PR landed is now gone?

I believe the command should be npm run test:e2e -- --update-snapshots. Try that and see how it goes.

My bad, that was even in the docs!  Thanks.

P.S. I am assuming that the difference in behavior between soft and hard resets that was troubling you before the end-to-end testing PR landed is now gone?

As far as I can tell, yes!

Ok, the last push should have updated the e2e tests so the Turtle tests pass.    I'm not sure what the issue is there.  Any insights appreciated.  I still need to add a unit test for the new math.ts functionality, right?
  1) [firefox] › scope.spec.ts:130:5 › Scope › refreshing the specimen ─────────────────────────────

    Error: locator.evaluate: Error: strict mode violation: locator('#canvas-container canvas') resolved to 3 elements:
        1) <canvas class="p5Canvas" data-hidden="true" id="defaultCanvas0"></canvas> aka locator('#defaultCanvas0')
        2) <canvas class="p5Canvas" data-hidden="true" id="defaultCanvas1"></canvas> aka locator('#defaultCanvas1')
        3) <canvas class="p5Canvas" data-hidden="true" id="defaultCanvas2"></canvas> aka locator('#defaultCanvas2')

    Call log:
      - waiting for locator('#canvas-container canvas')

      131 |         const oldCanvas = await page.locator('#canvas-container canvas')
      132 |
    > 133 |         await oldCanvas.evaluate(canvas => {
          |                         ^
      134 |             canvas.classList.add('old-canvas')
      135 |         })
      136 |         await page.locator('#specimen-bar-desktop #refresh-button').click()

        at /test/e2e/tests/scope.spec.ts:133:25

  2) [chromium] › scope.spec.ts:130:5 › Scope › refreshing the specimen ────────────────────────────

    Error: locator.evaluate: Error: strict mode violation: locator('#canvas-container canvas') resolved to 4 elements:
        1) <canvas width="980" height="643" class="p5Canvas" id="defaultCanvas0"></canvas> aka locator('#defaultCanvas0')
        2) <canvas width="980" height="643"></canvas> aka locator('canvas').nth(1)
        3) <canvas width="980" height="643"></canvas> aka locator('canvas').nth(2)
        4) <canvas width="980" height="643"></canvas> aka locator('canvas').nth(3)

    Call log:
      - waiting for locator('#canvas-container canvas')

      131 |         const oldCanvas = await page.locator('#canvas-container canvas')
      132 |
    > 133 |         await oldCanvas.evaluate(canvas => {
          |                         ^
      134 |             canvas.classList.add('old-canvas')
      135 |         })
      136 |         await page.locator('#specimen-bar-desktop #refresh-button').click()

        at /test/e2e/tests/scope.spec.ts:133:25

  Slow test file: [chromium] › featured.spec.ts (1.0m)
  Slow test file: [chromium] › transversal.spec.ts (43.4s)
  Slow test file: [firefox] › transversal.spec.ts (41.5s)
  Slow test file: [firefox] › featured.spec.ts (36.6s)
  Slow test file: [chromium] › gallery.spec.ts (24.1s)
  Consider splitting slow test files to speed up parallel execution
  2 failed
    [chromium] › scope.spec.ts:130:5 › Scope › refreshing the specimen ─────────────────────────────
    [firefox] › scope.spec.ts:130:5 › Scope › refreshing the specimen ──────────────────────────────
  4 skipped
  83 passed (1.1m)

Any insights appreciated. I still need to add a unit test for the new math.ts functionality, right?

Oh I think I figured it out.  I left another numberscope running elsewhere.  Duh.

I still need to add a unit test for the new math.ts functionality, right?

Yes, new functions should have new unit tests for them.

Oh I think I figured it out. I left another numberscope running elsewhere. Duh.

Not sure what you mean; unit tests (except for firefox WebGL, which "refreshing the specimen" is not, run in a Docker container so they are in an isolated environment. Sure looks like something is going wrong. I will take a look in a bit if you haven't tracked it down.

Not sure what you mean; unit tests (except for firefox WebGL, which "refreshing the specimen" is not, run in a Docker container so they are in an isolated environment. Sure looks like something is going wrong. I will take a look in a bit if you haven't tracked it down.

Oh, hmm, yes, I was wrong, another instance of numberscope was not the problem.  I don't know why I'm having a problem -- it passed on github I see.  Anyway, just added a unit test for math.ts.  Maybe you could try testing e2e on this PR on your machine.

I am seeing the same two failures as you are. I will look into it.

OK, it seemed to be some kind of bad interaction with the "refresh a specimen" test and the change I made and you merged to a random featured visualizer as the default. So I switched that test back to always using a Random ModFill visualization, and it is passing reliably now.
However, you also didn't seem to have firefox snapshots for VFib Snowflake or Beatty DNA, so I added those two. But now it's critical before anything proceeds any further for you to verify that those snapshots match on your system.
Once you've checked that, if you think this is ready for review, just remove the "Draft" designation; or if you have other things you need/want to work on further, please go ahead when you have a chance.

it's critical before anything proceeds any further for you to verify that those snapshots match on your system.

Passes!  Yay!

OK, I am going to put further comments specifically about Turtle here rather than on joint emails. I believe there is significant additional speed up to be had here, for two main reasons:

If I am reading the code correctly, it is currently recomputing the entire path on a pan or zoom, when all it needs to do is redraw the path. This we should definitely fix.

It is currently drawing the path by a series of line segments. p5.js has no way of knowing that/whether these line segments have anything to do with each other. Worse, if I am correctly understanding p5.js's design and how WebGL works, when they are individual line segments, they are sent individually to the GPU, at which point it just processes the 2-4 triangles needed to make that segment, and then there has to be another cycle from the CPU to the GPU, and so basically you are getting no speedup over ordinary drawing in 2D mode that happens all on the CPU, because it's essentially dominated by CPU -> GPU communication, not by the actual computation.

So, what are the appropriate plans here?
A) Fix point 1 above. This should happen in any case.
B) Consider the reasons to have switched to WebGL. It was likely in large part motivated by my sense of getting pan and zoom "for free" without having to redraw. That turns out to be wrong, you do have to re-draw. On the other hand, it is quite simple to implement pan and zoom in terms of moving the camera, and then we can write all of our drawing code in fixed coordinates. I feel like that is a win in the clarity and simplicity of the code. So I am OK with sticking with WebGL, but if anyone on the team feels we should just go back to ordinary 2D drawing on the canvas, since the raw switchover likely bought us no performance, I am open to considering it.
C) However, we likely can take advantage of the GPU in two ways:
i) Create the path "chunks" that I discussed in an email. These then live on the GPU, and do not have to be re-sent to the GPU on each frame. That's a win.
ii) Rather than draw individual line segments, create mesh objects with polygonal paths via
sketch.beginGeometry()
sketch.beginShape()
for (v of path.vertices) sketch.vertex(v)
sketch.endShape()
const mesh = sketch.endShape()

This way, each chunk should be a single mesh, with half the vertices of a sequence of line() calls since the internal vertices are shared between successive segments, and the GPU should be able to parallelize the rendering of the path chunk. Note this should help some even with folding, to which neither of the other speedups (A) or (C.i) apply.
So my plan, unless there's enthusiasm for discussing otherwise, is to implement both of A and C (as a few commits) in this PR before we move on from this topic. I wouldn't have been motivated to do this, but for the significant usability issues Aaron was encountering (surprisingly since although it was jerky for both Kate and Glen, there was nothing even close to threatening the usability of the browser).
Cheers.

That all sounds excellent Glen, and I agree wholeheartedly.  I had thought about addressing A in any case, and incidentally, I had at one point tried drawing paths instead of individual segments, but that was at least naively incompatible with the "growing" paths (since you need to chunk them), and I wasn't aware of the rendering advantage.  Given the need for speed, it now seems worth it to build stored chunks for later use.  So I entirely agree with all this.  Thank you!

As for (B) I think the effortless pan/zoom is a big plus for WebGL, plus the option for 3D visualizers in future!  So unless it is itself causing a "speed-down" I vote we keep it!

OK, so far I have done (A) and (C.ii). I think it has helped a little; @Vectornaut and @katestange you are welcome to try it and see what you think. I will continue with (C.i) when I have a chance.

This has made a significant difference!  In chromium the same url (http://localhost:5173/?name=Beatty+DNA&viz=Turtle&domain=0+1+2&turns=79+0+45&steps=5+2+6&foldControls=true&folds=100+0+100&speed=100&bgColor=e9eee3&strokeColor=4b7a81&seq=OEIS+A001951&modulus=3) folds are around 35-45 fps instead of 10-20 fps before these changes.

Well that's great. I am a little confused though; how can folds be 35-45 fps when Turtle sets the framerate to 30? Also, how does one see the frame rate?

OK, well now I have the "chunking" in, and this is a much bigger win. Let Beatty run until the end. Then drag and zoom to your heart's content. It flies -- this was the kind of performance I'd expect from WebGL. In fact, for kicks I'm attaching a whole lot more Beatty DNA generated with the formula -- try it, you can still drag it all over the place with ease.

OK, that's all of the remedies, and indeed, they seem to have been worthwhile. So I'm passing the baton back. But while these things are fresh in my mind, here are some thoughts/suggestions for things that we could do, and perhaps should do. I recommend that we consider each one for either (a) implementation in this PR, (b) opening a beta or later issue, or (c) deciding we don't want that.

Some Turtle paths are definitely long and skinny, but their major axis might not happen to align with the long dimension of the viewing rectangle. Should we either (a) allow rotating the view (would be trivial by rolling the camera, I believe), and if so, what would be the command/gesture for that, or (b) simply have an "initial direction" parameter (right now it is always 0)? One or both (a rotation gesture could update a parameter, even) would be especially useful with say Thue Trellis, it's not well aligned with the viewport.
For animation (folding), it's not quite clear to me why one can animate turn angles but not step lengths. Should that asymmetry be removed? If we did, it seems to me that the vector of step length deltas should be called "stretches" to be analogous with "folds".
Since things are faster now, do we want to reduce the amount of warnings about sequence lengths when folding and/or loosen the throttle slightly?
Turtle still has the default behavior that when the screen is resized, it redraws from the beginning. But since we are now saving the entire path and in fact can redraw it very efficiently with our chunks, should we change that to simply redraw and keep marching on with the current path, in case of resize?
The current behavior is that "missing" entries in the step lengths/turn angles/folds are filled with 0. I think it would be more useful -- actually a feature -- if missing entries were made to be replicas of the last entry instead. That way if you want all steps to be length 42, you just specify a single 42. (And I think there is no need to warn in the case that there is just one entry in a vector but multiple domain entries -- it is a legitimate abbreviation for make them all the same. If you think it's needful, we could add an "info" status level for possibly helpful messages that would appear in green boxes.)
In a related vein, the Gallery "Wait For It" and "Thue Trellis" visualizations now warnsbecause the default fold value is '0 0 0 0 0' but they have fewer domain entries than that. I think the default fold value should just be '0', meaning all zeroes. In any case, on this one, I think something must be done in this PR so that no Gallery specimens show warnings.

Awesome!  I'll work through your list.  It's amazingly fast now!

I pushed two commits.  I've added stretching (which was easy).  I also added rotation, as a right-mouse click left-right gesture.  We could change that.  It calls on camera.roll() and if you could check on the typescript issue I'm having there, that would be a help.

Also, I'm seeing some inconsistent behaviour in the drag functionality.   Sometimes it works correctly, and sometimes it scales wrong (the image moves faster than the mouse).  The wrong behaviour seems to occur after a relaunch of the server, and the right behaviour seems to sometimes kick in when I've been modifying files and hot loading.  Are you seeing any weird behaviour with the drag/pan?

if you could check on the typescript issue I'm having there, that would be a help.

What's the problem? If you don't cast to any, it makes some error about calling roll? The problem could be missing types in the p5 type declaration package. If you notice elsewhere in the file, there were no types for beginGeometry() and endGeometry().

Are you seeing any weird behaviour with the drag/pan?

Indeed. I can reliably reproduce it by npm run dev, then opening the frontscope, visiting the Wait For It from the gallery, then reloading the page, and then as soon as it's completed its first little loop, click and drag it quickly in some direction. The image "slings" farther than I dragged it.
But most of the time it works.
If the procedure I outlined above also reliably exercises this problem for you also, then one of us should debug it on that basis -- let me know which of us you'd like to do it.

What's the problem? If you don't cast to any, it makes some error about calling roll?

Yes, it says camera has no 'roll'.

Indeed. I can reliably reproduce it by npm run dev, then opening the frontscope, visiting the Wait For It from the gallery, then reloading the page, and then as soon as it's completed its first little loop, click and drag it quickly in some direction. The image "slings" farther than I dragged it.

No, what I'm seeing is happening when I move slowly also, it behaves exactly as if the scale variable is incorrect.  Like any mouse movement of distance x will move the image a distance 2x. But it's intermittent, which is what's so odd.  Sometimes it's fine.  Changes from fine to not-fine on reloading the page.

What's the problem? If you don't cast to any, it makes some error about calling roll?

Yes, it says camera has no 'roll'.

OK, I verified that the p5 type declaration package is indeed missing "p5.Camera.roll" and filed an issue on the @types/p5 package. But as there are 17 open issues on p5, dating back three years, I would not expect it to be fixed in the foreseeable future. Perhaps at some point we will deem it worthwhile to try to contribute to that package, but I am unclear how to even get started on that.
In the (likely permanent) meantime, I'd suggest just adding a @ts-expect-error annotation like I did for beginGeometry with a comment about the problem, rather than casting to any, since we are working around a problem in the package, not around some inconsistency in our typing as casting to any would suggest. Whoever gets to it first should make this change.

what I'm seeing is happening when I move slowly also, it behaves exactly as if the scale variable is incorrect. Like any mouse movement of distance x will move the image a distance 2x. But it's intermittent, which is what's so odd. Sometimes it's fine. Changes from fine to not-fine on reloading the page.

Similar enough that I think it's likely the same problem. As it is solidly reproducible for me via reloading the page, I will try to debug on this end. Maybe the drag function is getting called twice per frame somehow? Will keep you posted.

Dragging: this seems really pernicious. I have verified that mouseDragged is being called more than once with the same event (same in the sense of being the same JavaScript object). And it appears that some of those calls are "extra" in that they do not correspond to additional mouse motion. On the other hand, it seems that if we ignore all mouseDragged calls with the same event as the prior call, then we don't move the camera enough! (In other words, it seems as though sometimes p5 is re-using the same event for additional motion, and sometimes it is re-calling with the same event even though there is no further motion.) This all seems crazy.
I don't see much alternative besides not using mouseDragged to track drags ;-( I will try to work something out.

OK, I switched P5GLVisualizer to only rely on mousePressed(), mouseReleased(),  and the mouseX and mouseY properties for handling drag to pan and right-drag to rotate. The downside is that then derived Visualizers must call .handleDrags() in their draw() function -- but this is no different than having to call the p5 function sketch.orbitControls() in the draw() function to get the standard rotate/zoom in 3D behavior that p5 WebGL mode offers, so I don't feel to bad about it.  The controls seem stable and smooth now.
There is a problem in that when Histogram is rotated (e.g. by 90 degrees), the mouseovers no longer correspond to the bar positions. That definitely needs to be fixed -- either in this PR, or in a new issue with an alpha or definitely no later than beta milestone. An alternative would be to not allow rotation in Histogram; I am open to that, too.
Anyhow, @katestange, you can proceed with whatever you have remaining before this is ready for "final review".

An alternative would be to not allow rotation in Histogram; I am open to that, too.

I'm not sure Histogram strictly needs any of the P5GL functionality, but I can see the argument for zoom/pan.  But rotation doesn't feel like it really has a use case.  I would be ok with disabling it.

The controls seem stable and smooth now.

Thank you so much for figuring this out!

The controls seem stable and smooth now.

Thank you so much for figuring this out!

I didn't really figure it out; I just sidestepped the problem by not using mouseDragged.
I have no objection to initial position and bearing, but there is now a sort of odd redundancy between them and pan/rotation. In other words, each can be implemented in terms of the other: you could leave the camera alone and just change the initial position to pan, or you could always start from (0,0) and set the initial camera position to simulate a different starting position. And the analogous things for bearing. However since recomputing the path is expensive whereas now moving the camera and redrawing is relatively cheap, there is a performance argument for simulating starting position and bearing just by setting the initial camera position and orientation. And that further suggests that as one pans and rotates, these parameters should auto-update to the values that would produce the current view if started with those parameters and then left alone. I will let you contemplate these points and decide whether any action should be taken.

I have no objection to initial position and bearing, but there is now a sort of odd redundancy between them and pan/rotation.

Here's a little more on the underlying philosophy behind this issue.  I'm interested in your thoughts, since this is perhaps relevant to all P5GL visualizers, and it may be valuable to have consistency across Numberscope.  There seem to be two philosophies we could take:
(1) We could think of the inbuilt GL functionality (zoom/pan/rotate/etc) as part of the "viewing process" of the specimen (the knobs on the microscope).  This is something that a user may play with without the expectation that this changes the URL, or is saved in any way.  When we reload, these changes go away.  (Resizing the browser window is already sort of in this category -- reloading doesn't reset browser window size, but neither does browser window size persist in the URL.)
(2) We could think of the inbuilt GL functionality as a way to interact with the specimen's definition (the slide we choose to put into the microsocrope).  Then the user expects this to be saved, persist on reload, be part of what is shared when sharing a specimen, etc.
For Turtle in particular, we might view starting state as (2) but zoom/pan/rotate as (1).  They have the same effect, but they come from different philosophical places.  So someone might wish to save the rotation and placement they like best for sharing, or they might just be zooming rotating and panning to look at a detail momentarily for their own curiosity.
In terms of user experience, there seem to be several considerations:
(1) when using a mouse to rotate/pan/zoom, I think the user more likely expects from experience to think of this as a "viewing process" aspect (not persistent on reload for example).  Think of 3d graphs you can rotate or 2d graphs you can pan in a math software like Desmos, for example.  Rotating or panning doesn't change the code, and reload resets the view.
(2) when changing a parameter panel entry, the user probably expects this to persist, since this is true for all the others in the panel
(3) the same effect on the canvas might be expected to occur only one way (this is the redundancy issue you bring up)
These are in conflict with one another.
So now my own personal opinion is that I feel (1) and (2) trumps (3), in my own user expectation/experience -- this is just an intuitive feeling.  But I'm open to counterpoint.  And I think it's relevant to the wider Numberscope, not just Turtle, so we should probably discuss in some detail.
Also, you said "However since recomputing the path is expensive whereas now moving the camera and redrawing is relatively cheap, there is a performance argument for simulating starting position and bearing just by setting the initial camera position and orientation."  -- I think the implementation could be changed either way we go on the philosophy.

Fortunately we are meeting today so yes I agree we should discuss. The only point I will make in this post is that I think this issue is closer to particular to Turtle rather than general to all P5GLVisualizers. That's because it happens that Turtle has fairly natural possible parameters (initial position and bearing) that ultimately duplicate the effect of pan and roll. I think most P5GLVisualizers will not.

I just added some commits that address all the remaining issues raised in Glen's last-week list except "_Turtle still has the default behavior that when the screen is resized, it redraws from the beginning."
In particular, I've done some aesthetic improvement of gallery specimens, added a separate "uniform fold rate" option, added a bit in the test docs about updating snapshots (since it is coming up for me), removed the start position/bearing, loosened warnings, small code adjustments.

One issue I'm seeing is that VFib Snowflake featured e2e test is flaky.  It seems like maybe it's capturing one fewer/more frame sometimes?  Another issue that has happened a few times is that --update-snapshots doesn't actually produce all the snapshots needed.  This time it only made ThueTrellis-chromium (and not firefox).  Earlier it happened and Glen noticed some snapshots were missing and fixed it.  I don't know why that happens??

Glen, I'm having a tough time finding some time to wrap this up, but it's very close.  If you could adjust the resize issue, that would be great, then I'd be ok with marking it ready for review.  Otherwise I'll try to find time soon.

If you could adjust the resize issue, that would be great

Yes, I will add a commit or so that prevents Turtle from restarting on resize.

One issue I'm seeing is that VFib Snowflake featured e2e test is flaky. It seems like maybe it's capturing one fewer/more frame sometimes?

Yes, I agree. It seems like it is doing something like that. My personal feeling is to live with it (unless it becomes intolerable) until the new improved view controls are implemented, since they will likely solve the issue. If they don't, then I'd say we debug it then.

Another issue that has happened a few times is that --update-snapshots doesn't actually produce all the snapshots needed. This time it only made ThueTrellis-chromium (and not firefox). Earlier it happened and Glen noticed some snapshots were missing and fixed it. I don't know why that happens??

My personal theory is this is connected with the flakiness and so my attitude/approach is the same.

One other thing: Did you not like my suggestion of replicating the last entry of any of the vectors of step parameters (length, turn angles, fold, stretch) if there are more elements in the domain than in the vector? This would seem to pretty much cover the use case for "uniform folding rate" (just specify one fold, and it gets used over and over) and also things like '8 120 0 0 0 0 0' -- that could be just '8 120 0'. If you don't object, I will implement this, remove uniform folding rate, and adjust all of the gallery specimens to take advantage of this ability. Thanks for letting me know your views on this.

My personal feeling is to live with it (unless it becomes intolerable) until the new improved view controls are implemented, since they will likely solve the issue

OK, I am busily eating these words. (Nom, nom.) I simply could not get VFib or ThueTrellis to reproduce the snapshots you recently pushed, so I was forced to look into this. It turns out that when the URL requests "frames=64" in Thue Trellis and I load the page, I'm currently really only getting a varying amount of frames with mode 52 (and it's not hard for me to imagine your modal number of frames is different). The problem seems to be that somehow the specimen initializes itself seven (count them, 7!) times, with one or two calls to draw() each time that end up getting wiped out by the next initialization. So there is clearly something wrong going on at frontscope startup, which I am trying to debug.
Wish me luck, I will need it. I fear the inscrutable internals of Vue and/or p5 are about to need some serious scruting. ;-)

OK, it seems to be entirely caused by (A) the Vue reactive mechanism "fooling" our code into thinking that parameters had changed when they had not, and (B) the fact that when some parameters change (simultaneously, e.g. when loading a URL) we call the "parameterChanged" callback on the visualizer once for every parameter, potentially causing it to reinitialize itself on each call. I have already put in a safeguard against (A) in my local version of the code -- if a parameter seems to have changed, I convert its old and new values back to their string formats and only consider it a change if those are different. But for (B), on reflection, I cannot understand why the Delft team made multiple calls to parameterChanged when they also wrote code that meant each call to parameterChanged would by default reset the visualization. That seems a recipe for doing unnecessary multiple resets. I'm beat right now, but my plan is tomorrow to change this callback method to parametersChanged, which is always called just once, but with a nonempty list of the parameters that appear to have changed rather than just one parameter name.
Fingers crossed that will get us to where we produce more consistent results. I am hopeful because in my working copy where I just short-circuited the call, I indeed get a full 64 frames of drawing on every page reload. Will keep y'all posted.

Fingers crossed that will get us to where we produce more consistent results.

Wow, thanks Glen!  (And your "nom nom" made me laugh!)   I guess this is another example of the end to end tests being valuable (which is apparently equivalent to causing lots of extra work!).

One other thing: Did you not like my suggestion of replicating the last entry of any of the vectors of step parameters (length, turn angles, fold, stretch) if there are more elements in the domain than in the vector? This would seem to pretty much cover the use case for "uniform folding rate" (just specify one fold, and it gets used over and over) and also things like '8 120 0 0 0 0 0' -- that could be just '8 120 0'. If you don't object, I will implement this, remove uniform folding rate, and adjust all of the gallery specimens to take advantage of this ability. Thanks for letting me know your views on this.

Well, I guess I'm torn.  Recall we've talked about the "naive user" vs. "power user" issues.  Your suggestion seemed a bit like a "power user" feature, since the naive user is unlikely to read the descriptions of the parameters in detail and notice they can do this.  But then I thought, this is actually a very nice feature to have accessible and even default, and putting it up front makes it available to the naive user.  So while I have no objection to changing the default handling of "too short" folding arrays (repeat last entry instead of zero out remaining entries does seem more useful and so I agree this should change actually), and I recognize the redundancy you are pointing out, I'm now a bit fond of having an up-front way to turn on animation easily for the naive user.  Is there a solution which makes this "uniform fold" an easy default for the naive user while still being fairly clear that you can do individual folds and making it easy for power users?  I guess we could change the default folding array parameter to just one entry, and leave the individual folding for power users to discover by reading the parameter description in more detail.  But then maybe the functionality is too hidden?  I don't see a super obvious best solution.

All right, cleaning up the initialization and preventing rampant re-running of "assignParameters" indeed seems to have made the testing much more robust. Turtle is now not restarting the path because of a resize.
On the rule parameters: Since we agreed that the last value should be reused as needed, made that change. Since that technically made the "uniform fold rate" redundant, I removed that as well. And then as a compromise, I documented everything as though in any of the rule parameters (turns, steps, folds, stretches) you should either put a single value or a list positionally corresponding to the domain. Both of those things work pretty intuitively, and I hope that it ended up relatively clear and not too complicated/forbidding for an initial user. These are the two main, natural ways of using the parameters, anyway. (And in fact, you get some nice pictures in VFib Snowflake if you set the turn angle to a constant and just have the step lengths depending on the residue, so a fixed number for all steps is useful even in the turn angle parameter.)
Take a look at let me know if you're comfortable trying it like this. To me it seems like you can do most things you'd want to, without it being so overly complicated to start with.
I am going to assume that this is ready for review, and start reviewing it as if. If that's the case, please mark as ready for review; if there are other things you know of that you want done (whether you have the time to or not), please let me know.

Added a rough check of domain appropriateness, to address #76.

Added an error for the sake of #412.

OK, I feel like I've gone through this pretty thoroughly and I am not seeing anything else of concern. So I think the Turtle overhaul is done (for now anyway, see #471 ;-) ). Yay! Merging.


Answer refutation

@bschaefl Hi, hopfield networks are very interesting and understudied indeed however, can you please answer this?
https://medium.com/@tunguz/trouble-with-hopular-6649f22fa2d3

They provided answer here: https://twitter.com/bschaefl/status/1532739442930245635


Automator: update proxy@master in istio/istio@master

Generated by Automator - 2022-10-20T18:53:57+00:00

@istio-testing: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

integ-security-multicluster_istio
3fa643805e8a29ce370f97682643a76f4a367016
link
true
/test integ-security-multicluster

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

/test integ-security-multicluster


Improve validation for generators that only support observables.

Adds validation for bax_generator to ensure no objective function is specified. Also improves error message for bayesian_exploration generator.

Oops, I forgot to update/run the bax tests. Will fix now.

Tests have been updated and should now pass.

Looks okay at first glance. You could at a test for expected failure when objectives are specified.

I'm happy with this, any other comments @ChristopherMayes? I'll accept it in a few hours if you have no issues, or you can


Feedback Button

Add A Feedback Button To The App.

Completed In Release: v1.1.1
Commit: https://github.com/markydoodled/Journal.it/commit/67b8afae476956cdb3a40048475087d681afd772


Add namespace to templates for resources where it's missing

I'm attempting to use the redpanda helm chart via kustomize, and I'm encountering the fact that some portions of redpanda's helm templates have an entry for namespace:  whereas others don't and rely on helm to install it based on --namespace implicitly. Unfortunately kustomize doesn't respect this behavior (issue). My thinking is that some portions of redpanda's helm charts support templating it, that it'd be nice to have this behavior be more consistent.
I understand that helm also hasn't fully settled on its stance on this either (issue).
Apologies if this was suggested before. I looked around, but it's hard to search for.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.


⚠ Jenkins Plugins Artifactory has degraded performance

In 690c685, Jenkins Plugins Artifactory (https://get.jenkins.io/plugins/artifactory/) experienced degraded performance:

HTTP code: 200
Response time: 1331 ms

Resolved: Jenkins Plugins Artifactory performance has improved in 3c14cb2 after 12 minutes.


Adding Standard_Microsoft sku in CDN pricing tier

Description
Regenerating CDN SDK based on latest swagger spec.
https://github.com/Azure/azure-rest-api-specs/pull/3628

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

[ x] https://github.com/Azure/azure-rest-api-specs/pull/3628
[ x] I have read the contribution guidelines.
[ x] The pull request does not introduce breaking changes.

General Guidelines

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

Testing Guidelines

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

SDK Generation Guidelines

[x ] If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
[ x] The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
[ x] The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

@azuresdkci Retest this please

Logs added.
Thanks

@dsgouda I'm not sure how to do this on GitHub. Could you help?

The easiest way I can think of is:

Rename your local branch to something else and copy over all the files you changed here to a temp directory
Checkout psSdkJson6 on your local repo, pull down latest upstream changes and create a new branch with the same name as this, copy over the files from temp directory here, commit the changes and force push here

Hi, @dsgouda , It appears that I was working on branch psSdkJson6 and I am not sure how to make it clean again.
Do you think it is a good idea to restart from the very beginning and create a new PR?

That would be the safest bet. Please make sure you save your changes somewhere while doing so

@dsgouda, I created a new PR https://github.com/Azure/azure-sdk-for-net/pull/4730.
Thanks


[Python]Set pickle library at the Pipeline creation stage

Fixes: https://github.com/apache/beam/issues/21615

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[ ] Choose reviewer(s) and mention them in a comment (R: @username).
[ ] Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

R: @ryanthompson591

This is great if it works.
If I recall correctly, I tried a similar fix in the past and it did not work.
The way to know if you got it right (where I didn't) is to raise an exception in dill_pickler on any call. Then run the pipeline. If the dill pickler is never called then you know this workaround worked.
I think what was happening is before the pipeline init method got called, some of the transforms were pickling themselves.

I can run some pipelines with raised exception on dill, on portable runners to see if it works

I was able to run word count example with this change using cloudpickle along with raised exception on the dill dumps call. @tvalentyn do you suspect pickling happens before beam.Pipeline init call?

I don't know off the top of my head when that can happen. But it's possible to detect that this happens and print a warning.

if it happens, it is also possible that we may have to set this flag in multiple places (which is less ideal)

or we can do it during the instantiation of pipeline options. Let me try that.

I wouldn't do that in PipelineOptions

I think this line snippet would be sufficient. I ran the wordcount example and one RunInference example using cloudpickle . Worked fine.

Where I was seeing it was PTransformWithSideInputs.
https://github.com/apache/beam/blob/4522f4ce916223afb3c4edd02a7be6018d0ce5a4/sdks/python/apache_beam/transforms/ptransform.py#L864
If your code happens before that is called then I think we are golden.

Do you have an example that we can test it with?

ParDo instantiates from this class and in the wordcount example, we use a ParDo in the pipeline. So this should work.
https://github.com/apache/beam/blob/4522f4ce916223afb3c4edd02a7be6018d0ce5a4/sdks/python/apache_beam/transforms/core.py#L1386

R: @tvalentyn

Thanks, @AnandInguva !