Should Midstate contain the lenght?

I don't understand why Midstate doesn't contain the length. It seems to be absolutely crucial part of it - you always have to track it, provide it to hash engine...
Is there some fundamental reason to not add it?

If you are trying to go from a midstate back to a hash engine, then yes ... but then how woud the Midstate type be different from the Engine type?
What I'm getting at is, the reason we expose the "midstate" at all is for applications that either don't need, or already have, the length.
Do you have an application that needs both?

how woud the Midstate type be different from the Engine type?

Not containing the buffer which is presumably what the type is trying to avoid. So you only store midstate+len in the binary and create new HashEngine by copying the values.

Do you have an application that needs both?

Literally sha256t tags used in Taproot. Sure, they all have magic constant 64, but this constant needs to be stored somewhere and having it stored separately is awkward.
So perhaps, what are the other cases that use Midstate and do not need length at all, not even implied?

Why does 64 need to be stored anywhere? It is a platonic ideal.
The other use of midstate I'm aware of is the "fast merkle tree" construction in Elements, which exploits the fact that intermediate hashes are always exactly hashes of exactly 64 bytes, so by simply taking the midstate rather than completing the hash, you can do one sha2 compression rather than two, doubling the speed of your hashing. Again, the number 64 is fixed and therefore does not need to be stored anywhere.

The instructions in the code still contain the constant 64 somewhere. Can you refer some more documentation about fast merkle trees? I don't understand how it works from your description.

I guess 64 is written implicitly as the sum of two 32s ... but the code is here https://github.com/ElementsProject/elements/blob/master/src/primitives/txwitness.cpp#L45-L105
In your imagined API, if the user has input a number of bytes which isn't a multiple of 64, what would the length field of the midstate be? Would we round down to the nearest multiple of 64? Panic? Return the actual value? Note that in any case it would be impossible to reconstruct the actual state of the hash engine from this data. It seems like the effect of all these choices is to obfuscate low-level crypto code, which should only be used by cryptographers anyway.

Literally sha256t tags used in Taproot. Sure, they all have magic constant 64, but this constant needs to be stored somewhere and having it stored separately is awkward.

Indeed, it is supplied externally when creating tagged hashes as 64 as in the sha256t_hash_newtype macro.

I don't understand why Midstate doesn't contain the length. It seems to be absolutely crucial part of it - you always have to track it, provide it to hash engine...

Yes, you are correct that it would be insufficient information to create a midstate. But then, as Andrew mentions, you would also need to unprocessed bytes incase they are not a multiple of 64, but at that point, it's the same HashEngine

Yeah, panic on non-multiple of 64 as we already do. But we could also store it as block_count and then multiply by 64.

Why would it be wrong to use more than one block if someone actually did hash more than one block?

In that case shouldn't we just enforce the length to be always 64? So HashEngine::from_midstate wouldn't take length argument, it would check in to_midstate that length is really 64, perhaps name the Midstate raw bytes constructor with _unchecked suffix...

That would be a frustratig limitation for people who just want to check what the midstate is, e.g. for debugging purposes.

Ok, sure.
Though I'm still unconvinced of the value of asserting on creating a midstate. This seems surprising and I have no idea what we'd tell somebody if they hit it and asked what it was doing. It would make sense to assert when  trying to convert one back into a hash engine, which is when something would actually go wrong for a non-64-byte midstate.

Midstate does not guarantee length of 64. But when reconstructing hash engines from midstates (which is a low-level thing which should always be encapsulated), in all cases we require that the midstate came from a length of 64.

I'm proposing to change it to move the invariant to Midstate instead.

I think that's confusing because it's not really consistent with what "midstate" means. It's an extra requirement that only exists because of one particular use of the midstate.
I'm not entirely opposed but this does feel like we're adding complexity to low-level crypto code and I still don't understand really what the point is.

Maybe we should rename the struct then?
I actually think that having magic 64s everywhere adds complexity. We need to pass the same constant at every from_midstate even though it should be implied, the const in sha256t_hash_newtype macro has to be an awkward tuple...
If we do this we only need one check and then everything else can rely on it.

I think, of all the proposals here, the one I like best is to extend the midstate type to include an actual midstate field with the bytes as well as a block_count field. This is outside the normal definition of "midstate" but it's very clear from the type what is meant.
I wouldn't mind also having an assert in Engine::from_midstate, though I guess there wouldn't really be any reason for it since we could just accept any value for block_count and multiply by 64.

Renaming the type to Midstate64 or something would be my second choice.

If there's a block count then I assert in from_midstate doesn't make much sense. If we want to discourage people from rolling their own crypto we should do it at compile time.
If we're going to rename it I like Midstate64.

Well, there's not much we can do about home-rolled crypto -- off the top of my head, Elements uses these midstates heavily in their "fast merkle root" construction, miners use them for asicboost, and both of these are too "home-rolled crypto" for me to want them included in the library, but not "home-rolled crypto" enough that we should outright try to undermine them.
We could put a scary doccomment on Midstate if there isn't one already, but I think that's as far as we ought to go. Feature-gating it is probably more inconvenience than it's worth.

Want to agree to add the block_count field?
This would give us a nice opportunity to practice doing a major release and do the semver trick, actually, since only one obscure type would be affected.

OK, agree. I'm not sure if there's a point in semver trick if we're planning large breaks anyway.

This is stale now, right?

We have bytes_hashed in Midstate.
We don't take a length arg in Hash::from_midstate anymore.
We have can_extract_midstate, midstate, and midstate_uncheckde

I don't fully grok the original motivation but it seems we covered this?

I believe so. I'm going to close this. But as I mentioned in the other issue I'd like to get some releases out so we can play with this before declaring "done". In particular for midstate stuff I think rust-simplicity will exercise the API pretty well.


Fix -mfloat-abi=soft compilation for ARM with OpenCL target

Currently, any extra attributes of the llvm target are not being passed through to LLVMModule during ModulePackImportsToLLVM. This will cause compilation to fail with error: /tmp/tmpctakbpk1/devc.o uses VFP register arguments, output does not when the target is opencl and the target_host requires -mfloat-abi=soft. This regression was introduced by #4657.
There are two reasons why:

python/tvm/runtime/module.py uses "_get_target_triple" to get the target triple to use for ModulePackImportsToLLVM. However, the implementation of that function returns tm_->getTargetTriple().str() which will only have the standard <arch><sub>-<vendor>-<sys>-<abi> and is missing any extra flags or attributes such as -mfloat-abi=soft.
Since the module was created by CodegenBlob, it doesn't store anything in the "tvm_target" module metadata and therefore module_->getTargetTriple() is called which again doesn't have the extra flags and attributes: https://github.com/apache/incubator-tvm/blob/master/src/target/llvm/llvm_module.cc#L254

This PR fixes those problems by:

Adding important attributes back into  _get_target_triple (only mfloat-abi for now). This part doesn't seem ideal to me. Any thoughts on how to improve? Can we store the user's full target string earlier on?
Saving full target triple string in "tvm_target" module metadata flag during CodeGenBlob so that LLVMModuleNode::Init(std::unique_ptr<llvm::Module> module, std::shared_ptr<llvm::LLVMContext> ctx) can retrieve it: https://github.com/apache/incubator-tvm/blob/master/src/target/llvm/llvm_module.cc#L247 This matches what LLVMModuleNode::Init(const IRModule& mod, std::string target) already does.

Discuss post: https://discuss.tvm.ai/t/opencl-target-for-32-bit-arm-linux-android-broken-after-pr-4657/7252
Fixes https://github.com/apache/incubator-tvm/issues/6019

Looks like some unrelated failures in ci? tests/python/frontend/tensorflow/test_forward.py::test_forward_nms FAILED [ 50%]
E     let %x7: Tensor[(5), int32] = vm.reshape_tensor(%x5, meta[relay.Constant][6], meta[relay.attrs.ReshapeTensorAttrs][0]) an internal invariant was violated while typechecking your program [00:42:28] /workspace/src/relay/op/vm/vm.cc:195: Check failed: tt: input must be tensor type

Yes, looks like the CI failure is not related

@kevinthesun CI is passing now


[SPARK-41271][SQL] Support parameterized SQL queries by sql()

What changes were proposed in this pull request?
In the PR, I propose to extend SparkSession API and override the sql method by:
  def sql(sqlText: String, args: Map[String, String]): DataFrame

which accepts a map with:

keys are parameters names,
values are SQL literal values.

And the first argument sqlText might have named parameters in the positions of constants like literal values.
For example:
  spark.sql(
    sqlText = "SELECT * FROM tbl WHERE date > :startDate LIMIT :maxRows",
    args = Map(
      "startDate" -> "DATE'2022-12-01'",
      "maxRows" -> "100"))

The new sql() method parses the input SQL statement and provided parameter values, and replaces the named parameters by the literal values. And then it eagerly runs DDL/DML commands, but not for SELECT queries.
The SQL config spark.sql.parameters.enabled turns on or off this new feature. It enables parameterized SQL by default.
Closes #38712
Why are the changes needed?

To improve user experience with Spark SQL via

Using Spark as remote service (microservice).
Write SQL code that will power reports, dashboards, charts and other data presentation solutions that need to account for criteria modifiable by users through an interface.
Build a generic integration layer based on the SQL API. The goal is to expose managed data to a wide application ecosystem with a microservice architecture. It is only natural in such a setup to ask for modular and reusable SQL code, that can be executed repeatedly with different parameter values.

To achieve feature parity with other systems that support named parameters:

Redshift: https://docs.aws.amazon.com/redshift/latest/mgmt/data-api.html#data-api-calling
BigQuery: https://cloud.google.com/bigquery/docs/parameterized-queries#api
MS DBSQL: https://learn.microsoft.com/en-us/azure/databricks/sql/user/queries/query-parameters

Does this PR introduce any user-facing change?
No, this is an extension of the existing APIs.
How was this patch tested?
By running new tests:
$ build/sbt "core/testOnly *SparkThrowableSuite"
$ build/sbt "test:testOnly *PlanParserSuite"
$ build/sbt "test:testOnly *AnalysisSuite"
$ build/sbt "test:testOnly *ParametersSuite"

@cloud-fan @entong Could you take a look at this PR, please.

What is the relationship between this PR and #38712? Why do we have two PRs?
If this PR is superceding #38712, can we continue discussion here on which identifier to use based on my last comment on the old PR?

What is the relationship between this PR and https://github.com/apache/spark/pull/38712? Why do we have two PRs?

@xkrogen The previous one was a draft PR. This one supersedes it.

can we continue discussion here ...

Yep.

please let me know if you have a better source ...

As I can understand the SQL standard, it doesn't restrict identifiers in our case. The standard requires the : marker in another cases:

Names of host parameters in externally-invoked procedure

https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#host-parameter-name

Embedded variable names

https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#embedded-variable-name
cc @srielau

I managed to find a better SQL standard reference in the form of SQL: The Complete Reference (2003), which has an entire chapter devoted to Dynamic SQL (beginning from page 547). You are correct regarding colons; those are indeed host variables. The standard for PREPARE/EXECUTE refers to positional arguments using ?; this makes sense given how widely support that syntax is (as mentioned in my last comment).
However, I think it is safe to make an argument that what you are trying to add here is not, in fact, dynamic SQL, but rather host variables. We have no notion of precompiled statements with PREPARE/EXECUTE, which is the hallmark of dynamic SQL. Rather we have a query with a few "blanks" to be filled in, and we want to provide those blanks from the host execution environment. That is exactly what a host variable is for, right? You happen to be proposing an API (in Scala) that provides the values of the variables as map, instead of pulling them directory from the JVM execution environment, but from a SQL syntax perspective they are identical.
i.e. you propose this example (syntax updated to :):
  spark.sql(
    sqlText = "SELECT * FROM tbl WHERE date > :startDate LIMIT :maxRows",
    args = Map(
      "startDate" -> "DATE'2022-12-01'",
      "maxRows" -> "100"))

This SQL syntax is fully compatible with the standard for host variables, it's just that the Scala interface is slightly different. This would not preclude us from supporting the following in the future using the exact same SQL syntax:
  val startDate = "DATE'2022-12-01'"
  val maxRows = "100"
  spark.sql(sqlText = "SELECT * FROM tbl WHERE date > :startDate LIMIT :maxRows")

Both are fully compatible; one is explicit and one is implicit.

I could just as easily say that we should choose : to make it easier to migrate from Redshift ...

How about to support both @ and :? In that way, we cover and encourage more users to migrate on to Spark SQL.

Regarding supporting two syntaxes vs. standardizing on one, I don't have much of an opinion, but I would be curious about how Spark has handled such decisions in the past when there are competing dialects. @cloud-fan, any thoughts here?

@xkrogen After offline discussion with @cloud-fan @srielau, we decided to change the parameter marker to : in the PR.

After switching from @ to :, I got many parsing errors (see link):
SchemaPruningSuite.prune schema by the requested fields
org.apache.spark.sql.catalyst.parser.ParseException:
[PARSE_SYNTAX_ERROR] Syntax error at or near ':int'(line 1, pos 10)

== SQL ==
a struct<a:int, b:int>, b int
----------^^^

@cloud-fan Could you look at the PR one more time, please.

@MaxGekk Could you sketch out how this API could be extended to INOUT and OUT parameters in the future?

@MaxGekk Could you sketch out how this API could be extended to INOUT and OUT parameters in the future?

@srielau This feature precedences INOUT/OUT parameters. Apparently, the sql() method will substitute only parameters specified in args, and will leave others as INOUT params. Till the INOUT feature is not implemented in OSS, Spark fails w/ UNBOUND_SQL_PARAMETER when it faces to an param w/o provided argument.

@cloud-fan @entong Could you review this PR one more time, please.

Merging to master. The last commit is minor one.
Thank you, @cloud-fan @xkrogen @entong @srielau for review.


grpc gateway: refactor http server to proxy for grpc requests without connection

Motivation
Grpc gateway provides interface for registering proxy handlers without an additional connection to the grpc server. This way grpc gateway simply marshal grpc response to json and sends it to client, removing connection between HTTP server and grpc server in the middle.
Changes

pass list of services to HTTP server to register

Test Plan

[x] Explain motivation or link existing issue(s)
[x] Test changes and document test plan
[x] Update documentation as needed

DevOps Notes

[x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
[x] This PR does not affect public APIs
[x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
[x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors merge


Add this app to share menu extension when any folder is clicked in macos

Is your feature request related to a problem? Please describe.
No its not related to any bugs but when I click share it doesnt show up.

Describe the solution you'd like
I tried to look it up, it says you need to just do some changes in the app bundle to include it
https://stackoverflow.com/questions/76404963/i-want-to-add-my-electron-app-to-macs-share-menu-extension-when-any-folder-is
Describe alternatives you've considered
I dont seem to find any alternative. But since most of us rely on it heavily it would be great if you guys can add it
Additional context
No response

Yeah, I also want it appear on share menu on Windows.

Any update on this?

Duplicate of https://github.com/localsend/localsend/issues/586. will be in the next release (v1.16.0)


Some activities not showing on Stravistix!

Bug description:
The activities from 13may to 25 may aren't showing on my stravistix multisport fitness tool while all other strava activities are shown, it's quite annoying cause the result is a graph that's not realistic at all so it become totally unuseful
Actual Behavior:
Recent activies are showing, just the ones from 13-25 may period aren't
Expected Behavior:
i expect to see all my activities so i can obtain a realistic graph
Steps to Reproduce:

...
...
...

Chrome version
58.0.3029.110 (64-bit)
Plugin version:
5.7.2 Stable
Activities links?:
https://www.strava.com/activities/986286820
https://www.strava.com/activities/994412597 and many others
Console errors? (press F12 to see developer console, and copy paste here):
no errors registred
put console errors here if exist

Link screenshots or youtube video link if necessary:

as you can see there's a big hole in period 13-25 , i registred a lot of activities during that period

As written in feature infos/note you need hr monitor

Here you will be able to view your fitness trend using all activities where you held a heart rate monitor.

Hold one next time ;) :)


Profiler should measure commit phase durations

View formatted RFC
Implementation: https://github.com/facebook/react/pull/17910

Implementation PR has been accepted and merged, so I'm going to merge the RFC.


Update expand_dims and repeat

Rename dim to axis
Repeat works on any dimension

I'll do the NTM part in a separate PR.

Relaunching the CI tests.


[gsl-lite] Updated to 0.26.0

https://github.com/martinmoene/gsl-lite/releases/tag/v0.26.0

Thanks!


Plot() container not working?

I am trying to use the Plot() container, as advertised on the wiki, but I get an error.
julia> let p = Plot(), x=linspace(-2pi,2pi,100)
           add(p, Curve(x, sin(x)))
           Winston.display(p)
       end
ERROR: MethodError: `limits` has no method matching limits(::Winston.Plot)
Closest candidates are:
  limits(::Winston.Plot, !Matched::Graphics.BoundingBox)
  limits(::Any, !Matched::Any, !Matched::Any, !Matched::Any, !Matched::Any, !Matched::Any)
 in compose_interior at /home/yves/.julia/v0.4/Winston/src/Winston.jl:1447
 in page_compose at /home/yves/.julia/v0.4/Winston/src/Winston.jl:1848
 in page_compose at /home/yves/.julia/v0.4/Winston/src/Winston.jl:1836
 in anonymous at /home/yves/.julia/v0.4/Winston/src/tk.jl:27
 in display at /home/yves/.julia/v0.4/Winston/src/tk.jl:35
 in display at /home/yves/.julia/v0.4/Winston/src/Winston.jl:2855
 [inlined code] from multimedia.jl:151

What am I doing wrong?

Try using FramedPlot() instead of Plot().

I don't want a framed plot. I want a plot without a frame. From the doc, it seems that Plot() is the way to go.

Okay, I'm sorry. Can you try, if bd3c9af0242585137968a77fe5975c4981239763 works for you?

@slangangular Yes, it works, thanks.


Ensure that if gson is provided to httptemplate, the content type is application/json

What use-case is this trying to protect the user from?

It's specifically trying to protect the weather collection module from other developers' refactorings of shared code.  If you construct the httptemplate with a gson object, but provide a content type of "/" (not a real content type, but handy for browsers) then gson will not be invoked from a call to  HttpTemplate.post(String fullUri, Object bodyToPost).  It's incoherent to provide a value for gson that is not intented to be used.  This scenario occurred, so I'm preventing it.
There appears to be some more sloppy use of "contentType" here where a passed-in parameter is mixed up with a field, maybe I should clean that up as well.

I had missed that is was just on the POST side of things that the assertion was being made.  Seems fine to me.

That's cleaned up. Probably should be a different PR, but there it is.


feat(driver): support for init_module, finit_module syscalls

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod
/area driver-bpf
/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap-engine-udig

/area libscap
/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:
This adds the support for init_module, finit_module syscalls.
Which issue(s) this PR fixes:

Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:

NONE

Please double-check these tests in sinsp:
[  FAILED  ] events_set.event_set_to_names_generic_events
[  FAILED  ] events_set.event_set_to_names_no_generic_events2
[  FAILED  ] interesting_syscalls.event_set_to_sc_set_generic_events

You will notice that they assert INIT_MODULE syscall as generic but from now on it won't be generic

LGTM label has been added.  Git tree hash: 812b91bcdef34acd373e0220c101f27f968d812e

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: Andreagit97, FedeDP, therealbobo
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS [Andreagit97,FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


Fail to generate the CTD dataset

[lsong10@bhg0031 bran]$ ./extract.sh
Downloading Pubtator dump
--2019-03-31 21:09:22--  ftp://ftp.ncbi.nlm.nih.gov/pub/lu/PubTator/bioconcepts2pubtator_offsets.gz
=> ‘/home/lsong10/ws/exp.dep_forest/bran/data/ctd/bioconcepts2pubtator_offsets.gz’
Resolving ftp.ncbi.nlm.nih.gov (ftp.ncbi.nlm.nih.gov)... <IP_ADDRESS>, 2607:f220:41e:250::7
Connecting to ftp.ncbi.nlm.nih.gov (ftp.ncbi.nlm.nih.gov)|<IP_ADDRESS>|:21... failed: Connection refused.
Connecting to ftp.ncbi.nlm.nih.gov (ftp.ncbi.nlm.nih.gov)|2607:f220:41e:250::7|:21... failed: Network is unreachable.
Converting data from pubtator to tsv format
usage: process_CDR_data.py [-h] -i INPUT_FILE -d OUTPUT_DIR -f
OUTPUT_FILE_SUFFIX [-s MAX_SEQ] [-a FULL_ABSTRACT]
[-p PUBMED_FILTER] [-r RELATIONS]
[-w WORD_PIECE_CODES] [-t SHARDS]
[-x EXPORT_ALL_EPS] [-n EXPORT_NEGATIVES]
[-e ENCODING] [-m MAX_DISTANCE]
process_CDR_data.py: error: argument -a/--full_abstract: expected one argument
split: extra operand ‘up’
Try 'split --help' for more information.
map relations to smaller set
awk: cmd. line:1: fatal: cannot open file positive_0_genia' for reading (No such file or directory) seperate data into train dev test positive train 50 500 positive dev 50 500 positive test 50 500 negative train 50 500 awk: cmd. line:1: fatal: cannot open file negative_0_genia' for reading (No such file or directory)
negative dev 50 500
awk: cmd. line:1: fatal: cannot open file negative_0_genia' for reading (No such file or directory) negative test 50 500 awk: cmd. line:1: fatal: cannot open file negative_0_genia' for reading (No such file or directory)

Sorry for the delayed response. It looks like a network issue caused the download of the initial file to fail: "Connecting to ftp.ncbi.nlm.nih.gov (ftp.ncbi.nlm.nih.gov)|<IP_ADDRESS>|:21... failed: Connection refused.". This is causing all of the subsequent errors to print because each of the following steps require this initial file.

I got another problem when generating the training data of the CTD dataset.
Here is the error message:
Reading in relation file {MY_PATH}/BRAN/data/ctd/curated_ctd_interactions.gz
Traceback (most recent call last):
  File "{MY_PATH}/BRAN/src/processing/utils/filter_pubtator_keep_format.py", line 45, in <module>
    with (gzip.open(args.relation_file, 'rb') if args.relation_file.endswith('gz')
  File "/usr/lib/python2.7/gzip.py", line 34, in open
    return GzipFile(filename, mode, compresslevel)
  File "/usr/lib/python2.7/gzip.py", line 94, in __init__
    fileobj = self.myfileobj = __builtin__.open(filename, mode or 'rb')
IOError: [Errno 2] No such file or directory: '{MY_PATH}/BRAN/data/ctd/curated_ctd_interactions.gz'


Unable to change the Morgan Fingerprints for Neighbour search in Aurora postgresql using RDKIT Catridge

Problem Statement:
We have a requirement wherein we want to perform neighbor search on SMILES (placed in database table) using RDKIT catridge. The database of our application is housed in AWS RDS as Aurora Postgresql (running version 12.8). We have the latest RDKIT extension installed in Aurora. The below functionalities are fully performed in the database.
The neighbor search functionality is housed on a Postgresql user defined function. The function is a simple one whose job is to take the schema name of the table, the smiles in question and the tanimoto_threshold. The function internally uses tanimoto_sml(morganbv_fp(mol_from_smiles(''))).
Bug:
It seems the Morgan fingerprints is set to 512 bits (default) and cannot be changed from that value even though its mentioned in the documentation.
We have used the below statement to change the fingerprint bits:
set rdkit.morgan_fp_size=2048;
But during execution, the above statement is raising a warning and the fingerprint bits remain at 512 bits. Warning as shown below:
WARNING:  parameter "rdkit.morgan_fp_size" cannot be changed
Our aim is to change it to 2048 before we perform the neighbor search. This change is done at session level.
On the same way we are also changing the tanimoto threshold parameter in the DB session within the function with the below statement:
set rdkit.tanimoto_threshold=0.7;
The above statement is executing fine without any problem and the threshold is indeed getting updated. But the fingerprint size cannot be altered.
We have followed the following documentation wile designing and developing:
https://www.rdkit.org/docs/Cartridge.html
https://sourceforge.net/p/rdkit/mailman/message/32599793/
But despite the documentation, the command is not behaving as expected. We also have the function based gist index created on database table (housing the SMILES) as mentioned in the documentation
I have a doubt that though the functionality is present in normal Postgresql but not present in Aurora Postgresql.
Kindly let me know the next steps.

Sorry for the embarrassingly slow reply.
The parameter to change fingerprint size was disabled by the AWS team since using it makes it very easy to end up with data problems: when comparing fingerprints in the cartridge all of them must be the same size.
There is a thread about this here: #6644

Removing the "bug" marker since this is a configuration change by the AWS team, not anything we did

Thanks Greg for clarifying the issue.


Update backup instructions for tar.xz

Update the backup readme for the new archive format and clarify some wording

Thanks @s-crypt ! The backup readme clarifications have been merged.


docs: update the fractional playground description

This PR

update the fractional playground description

Hey @agardnerIT, can you check out the scenario again?
https://deploy-preview-1131--polite-licorice-3db33c.netlify.app/playground/
I fixed the issue you reported and updated the description. The concept is somewhat complex, so please let me know if this description clears things up.


⚠ Digital Ocean - https://status.digitalocean.com/ has degraded performance

In 92818a8, Digital Ocean - https://status.digitalocean.com/ (https://status.digitalocean.com/api/v2/status.json) experienced degraded performance:

HTTP code: 200
Response time: 331 ms

Resolved: Digital Ocean - https://status.digitalocean.com/ performance has improved in f50fc13.


Transition in a partial causes an error

I've a component that displays text based on a number of factor.This uses partials to help with the expected output. However if I try and use a transition in a partial I get an error :( if I take the partial and have it directly as my template component it works as expected. 1alpha

Please provide a reproduction.

http://jsbin.com/lemobe/edit?js,console,output


Update ruby version to 2.3.0

Trello https://trello.com/c/tCwG3wqV/449-upgrade-frontend-ruby-dependency-small
Update ruby version to 2.3.0.
A new warning is appearing now when running tests:
[warning] The response contained in an RestClient::Exception is now a RestClient::Response instead of a Net::HTTPResponse, please update your code

This was already referred here:
https://github.com/alphagov/specialist-publisher-rebuild/pull/706/commits/9c395728b9f628113ba2c0319a2a2a2f1a8ad81e
It should be safe to ignore it, although it is not great to have such a lot of warnings in tests results.

Did you try updating gds-api-adapters to rest-client 2.0.0?

@andrewgarner is looking into upgrading gds-api-adapters at the moment, speak to him

As discussed in standup, I've amended this pull request to keep the deprecation warnings from gds-api-adapters. This is not ideal as they are very noisy when running tests, but the only way to silence them is to change the configuration of gds-api-adapters as recommended here, which means when there is a 404 or 410 an error gets raised instead of returning nil. Some of our code relies on a nil being returned, and we are unsure we are consistently testing for this nil being returned, therefore changing the configuration might break things silently. That's why keeping the deprecation warnings for the time being is preferable.
I am preparing a story to fix this by the 1st of October at the very latest as that's the date gds-api-adapters will adopt the new behaviour by default.

Here is the proposed story to fix the deprecation warnings / handle the new behaviour of gds-api-adapters.
Here is the proposed story to refactor the root controller.
@h-lame fyi ^ and thanks for the help yesterday.

@binaryberry refactoring root_controller is definitely something we've wanted to do for a while, but we've also been swallowing that pain on the hope we'll just 🔥 it entirely when we do migration of the formats it handles.

@h-lame Mmmh, good point. I will include this thought in the card, might not be worth doing.

Should we just update to 2.3.1 now it is available on the boxes?

What Graham said, otherwise 👍


Show "home screen" shortcuts?

I'm loving this launcher but can't figure out how to show Home Screen shortcuts like the kind that Firefox and Native Alpha create - in the apps list. These are Progressive Web Apps (PWAs), as described here: https://developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Guides/What_is_a_progressive_web_app
Is this currently possible, or on the roadmap?
Thank you!

These are currently not supported (as they are not accessed similarly to normal apps), but adding support for them is on the roadmap.

Great news. Thank you!


Functional tests

This change is mainly about functional tests but there's also one implementation change. Due to the fact that raw data from ceph-mon:admin relation require some logic to properly format into data that's useful for k8s templates, I added property ceph_context that performs this logic.
There's also a potential race condition due to #4 which may cause relation ceph-csi <-> ceph-mon:admin to not form correctly if that relation does not contain all the required data at the time when it's formed. This will be addressed, however for now the functional tests pass on properly deployed testing bundle. Current workaround is to remove and re-add ceph-csi ceph-mon:admin relation after ceph-mon units are in active/ready state.
Closes #6

Overall, tests lgtm. I wonder if the bundle should not reference specific revision numbers. That would mean that the latest published revnos would be used. OTOH, not specifying a revno may make it a bit more tricky to troubleshoot (except if "juju status" is written in the debug artifact of the CI env).

Sorry, which revision numbers do you mean? Because in the func-test bundle, the chars are listed with revisions.

@auria As we discussed yesterday, It's probably best to keep charm versions consistent with latest CDK release


[v4] Bug: React 17, React is not defined after ejecting

React version: 17.0.1
Steps To Reproduce

npx create-react-app my-app
cd my-app
npm start (Works like a charm)
npm run eject
npm i
npm start (ReferenceError: React is not defined)

Link to code example: https://codesandbox.io/s/ecstatic-wood-ou6px

The current behavior
I upgraded react to 17 and removed "import React"s from my project files (except index.js where react mounted). But I got "ReferenceError: React is not defined" in those files.
To investigate the issue behind this, I created a new react app, the new JSX transformation was working. But when I eject, it throws the same errors.
So;
Without react-scripts, new JSX transformation doesn't working.
The expected behavior
Components should render without declaring React (import React from "react") if React is in scope.

This is not an issue about create-react-app.
I don't use create-react-app in the project I had problem with jsx-transformation.
I have just mention to show you example and give more idea about issue.
Thanks for replying quickly.
Happy coding :)

I'm having this exact issue even without ejecting. I followed the migration guide, ran react-codemod, deleted node_modules.
The compiler says the code compiled successfully but getting the red screen in the browser.

@alpeer  I've already fixed it in PR https://github.com/facebook/create-react-app/pull/9858

@alpeer
Upgrade React 17 and remove import React statements won't work since React doesn't do any JSX transform, you need babel plugin for that (https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#manual-babel-setup).

the CRA ejection issue has been fixed in #9885 and needs to wait for the maintainer to publish a new version.
:)

use npm create-react-app my-app

React version: 17.0.1
Steps To Reproduce

npx create-react-app my-app
cd my-app
npm start (Works like a charm)
npm run eject
npm i
npm start (ReferenceError: React is not defined)

Link to code example: https://codesandbox.io/s/ecstatic-wood-ou6px
The current behavior
I upgraded react to 17 and removed "import React"s from my project files (except index.js where react mounted). But I got "ReferenceError: React is not defined" in those files. To investigate the issue behind this, I created a new react app, the new JSX transformation was working. But when I eject, it throws the same errors. So; Without react-scripts, new JSX transformation doesn't working.
The expected behavior
Components should render without declaring React (import React from "react") if React is in scope.

I followed the steps here to reproduce the error. But everything works fine.
I did not have to do step 5 though, it ran a npm install automatically.

React version: 17.0.1
Steps To Reproduce

npx create-react-app my-app
cd my-app
npm start (Works like a charm)
npm run eject
npm i
npm start (ReferenceError: React is not defined)

Link to code example: https://codesandbox.io/s/ecstatic-wood-ou6px
The current behavior
I upgraded react to 17 and removed "import React"s from my project files (except index.js where react mounted). But I got "ReferenceError: React is not defined" in those files. To investigate the issue behind this, I created a new react app, the new JSX transformation was working. But when I eject, it throws the same errors. So; Without react-scripts, new JSX transformation doesn't working.
The expected behavior
Components should render without declaring React (import React from "react") if React is in scope.

I followed the steps here to reproduce the error. But everything works fine. I did not have to do step 5 though, it ran a npm install automatically.
This means the issue isn't with create-react-app

I checked, and this is true. All the steps are working fine on my system (mac) as well. Maybe you should try to reinstall React on your computer.

Can't reproduce the error here either.  Second that Steps to Reproduce, step 5 is done automatically.

npm i and npm install is the same thing tho

Hi, I want to work on this issue, can you please assign it to me

Hi, I checked the code and tried to reproduce the scenario but it didn't give any error so went and checked the code provided in the first post.
https://codesandbox.io/s/ecstatic-wood-ou6px?file=/src/index.js
So, the problem here is when you remove the react imports then you need to de-structure and then import the react functions as an object.

`import React from "react";
import ReactDOM from "react-dom";
import App from "./App";
const rootElement = document.getElementById("root");
ReactDOM.render(
<React.StrictMode>

</React.StrictMode>,
rootElement
);`
So, after you remove the react import statement then you have to remove the React. syntax from the code else the JSX compiler will not read the react code.
So, after removing the import the code should be changed to below:

`import {StrictMode} from "react";
import ReactDOM from "react-dom";
import App from "./App";
const rootElement = document.getElementById("root");
ReactDOM.render(

,
rootElement
);
`
Just a Note: It's not necessary to import react but the useEffect, useState, StrictMode needs to be imported .

In simple Terms,
Import React from "react" means we are importing the React Object;
if we removed the import from React and then use <React.StrictMode> it will throw an error saying react is not defined. If you remove <React. then you will have to import Strict mode. see below:
console.log(React);
console.log(typeof React);////object
console.log(React["StrictMode"]); //since we didn't import React so React is undefined.
// undefined["some_key"] doesn't make sense right, and there's no PR required for the fix, it's expected behavior of JavaScript.
All the confusion is because React has said there's no need to import React library but we need to import everything besides import React.

So is this issue solved?

is this issue solved?

I tried all the mentioned steps and everything is working fine with my code.
Components are rendered without declaring a react statement.
After ejecting also the jsx transformation works fine.
i```
mport { StrictMode } from "react";
import ReactDOM from "react-dom/client";
import "./index.css";
import App from "./App";
const root = ReactDOM.createRoot(document.getElementById("root"));
root.render(

);

Not necessary to import react object just import the strictmode from react and everything works fine.


render mail preview server-side

consistent with actual rendering
removes dependency on mustache

So one potential UX improvement we could do with client side rendering would be a live updating edit form. I don't think that's super important, so I'm not against this change but wanted to make sure we don't want that feature in the future.

I expected live updating and was a bit astonished when I realized it wasn't there. For this specific project I would argue that getting rid of a dependency is more important than a small UX improvement. But that is not for me to decide.


[Bug]: Move to Postgres - 504 errors when importing, editing, or deleting large apps on DP

Is there an existing issue for this?

[X] I have searched the existing issues

Description
When attempting to import, edit, or delete a large app on the DP, we encounter 504 errors, along with toast messages indicating that the server is taking too long to respond. Occasionally, after refreshing the page, UI elements like buttons and the search bar on the homepage become misaligned. Additionally, the app takes an extended time to load.
Note: Performing these actions with the same app in release does not result in any errors, and no issues are observed.

Steps To Reproduce

Visit https://ee-4987.dp.appsmith.com/
Import the app using this file: license-manager-app - dummy pages.json.
Attempt to edit or delete the app.

Expected Results:

The app should import without any errors.
You should be able to edit the app without issues.
You should be able to delete the app without encountering any errors.

Actual Results:
When importing the app, the reconnect datasource modal does not appear, and an error is displayed. Attempting to edit or delete the app results in 504 errors, and the homepage takes over 5 seconds to load.
Public Sample App
No response
Environment
Deploy Preview
Severity
High (Blocker to building or releasing)
Issue video log
https://jam.dev/c/34ab29c1-4e45-429f-afd0-3b7db06693e6
Version
Cloud

@shadabbuchh is this still relevant after performance optimization?

@shadabbuchh is this still relevant after performance optimization?

@abhvsn performance has improved for the above actions after the optimization. We can close the issue for now. If I notice any performance issue, I'll reopen it again.

Closing this!


Fix inproc transport bug on client WritesDone after status

Found this bug when running internal tests on #23361

I think this bug isn't completely fixed yet, so I'm going to mark this draft again and remove the review request.

This now fixes the problem that I had seen internally but there is another potential problem with the inproc transport that might need to be fixed. I'll file a separate issue for that.

Can you create an issue that explains what the bug is?

Yup! I created issue #23398 .

Thanks for the approval! Squashing and rebasing for final tests.

distrib tests are showing yellow on this screen but showing green on details, don't know why it's taking so long to report but i'm going to merge since it's definitely complete and successful (as are all other tests).

If you are also fixing the other bugs, it might make sense to add a test that makes sure it stays fixed.


Make dashboard grid more granular

Currently the dashboard view has 12 columns and a certain fix height matching this. We should at the very least double this, maybe go to 48.
There needs to be a database migration script that will multiply the current positioning.

Has this change been implemented? I'm running Superset in a test environment (version 0.35.2). My dashboard view has 12 columns, but I do not see a way of increasing this.


don't JSON parse huge requests

Not sure what the best way to configure this would be, but I think it would be a good idea to be able to ignore JSON parsing on really large data chunks to prevent over-allocation of memory as a potential attack vector.

This library isn't really specific to websockets anymore, so this is out of scope


Benchmark how long it takes to tear down 15 CRs

Following up on #3 and #4, I would like to see how long it takes to delete all 15 CRs and have all the memcached pods go away. Basically the inverse of #3, but run this benchmark after #4.
My MacBook Pro i9

Operator Type
Run 1
Run 2
Run 3
Average

Ansible
23.69s
23.85s
17.94s
21.83s

Go
17.61s
17.64s
17.51s
17.59s

Helm
23.33s
23.89s
23.49s
23.57s

GitHub Actions CI

Operator Type
Run 1
Run 2
Run 3
Average

Ansible
18.09s
17.86s
17.98s
17.98s

Go
17.56s
17.64s
17.62s
17.61s

Helm
18.09s
18.16s
18.08s
18.11s

Amazon EC2 (m5.large, 200GB gp2 volume)

Operator Type
Run 1
Run 2
Run 3
Average

Ansible
17.97s
23.63s
18.03s
19.88s

Go
17.60s
17.47s
17.35s
17.47s

Helm
23.33s
23.21s
23.32s
23.29s

Should be as simple as:
- name: Un-deploy {{ memcached_cr_count }} {{ operator_type }} memcached CRs.
  k8s:
    state: absent
    definition: "{{ lookup('template', 'templates/memcached-cr.yml.j2') }}"

- name: B3 Wait for all {{ operator_type }} Memcached Pods to be gone.
  k8s_info:
    api_version: v1
    kind: Pod
    label_selectors:
      - "{{ app_label_selector }}"
  register: memcached_pods
  until: (memcached_pods.resources | count) == 0
  delay: 5
  retries: 120

I have summarized these benchmark results in a Google Doc that we'll be using to build up an AnsibleFest presentation.


wxwidgets: fix building 3.1 on darwin

Description of changes
Fix build failures on darwin by passing all the necessary dependencies and removing deprecated command line switches to configure.

Things done

Built on platform(s)

[ ] x86_64-linux
[ ] aarch64-linux
[x] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
22.05 Release Notes (or backporting 21.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[ ] Fits CONTRIBUTING.md.

@knl there is already a commit in Nixpkg to update wxwidgets https://github.com/NixOS/nixpkgs/pull/157848

@tfmoraes thanks, haven’t seen it, as I was not looking in staging. Once it reaches master, maybe this PR could be useful, since it fixes building on darwin?

@tfmoraes I updated the PR to make 3.1 build on darwin. Please take a look. I'm wondering should we carve out a dedicated version without gtk, just for mac?


Integrator in saturation

In steady state PID = Kp * error + Ki(previous integral + error) + Kd(error - last)
= 0 + Ki (previous integral + error) + 0
= Ki (previous integral + error)
So   output of PID controller at the steady state is equal to integral term.
A problem arises if the regulator is in the saturation zone for a very long time.
Therefore, if the regulator is in the saturation zone, it is necessary to reduce the integral component.
I tried this but my knowledge of python is very low.
Instead of self._i_term += self._ki * error * delta_time I  set  this code
        if self._last_output == 0 :
           self._i_term += 0
        elif self._last_output == 100 :
           self._i_term += 0
        else:
           self._i_term += self._ki * error * delta_time

Maybe add this fix to the main plugin?

Basicly what you mean is that the pid should not do anything if reaches 0 or 100? Doesn't he already do that? The min and max values are constrained, it should not raise above the maximum, or lower bellow the minimum!

I'm also having extreme saturation problems with the integral component, I believe

Basicly what you mean is that the pid should not do anything if reaches 0 or 100? Doesn't he already do that? The min and max values are constrained, it should not raise above the maximum, or lower bellow the minimum!
No  -  if  rise 0  or 100 it  must stop adding  integral   for   example   we control temp  of water.  and  in stable

When the control action on the object reaches saturation, the feedback is broken (the control action does not depend on the state of the system).
Even with windup, the integrator continues to integrate, the signal at its output grows, but this signal does not participate in the regulation process.
I suggest turning off the integration when the regulator is in saturation.

I have seen that behavior, turn off the integrator, when the system is saturated!
You could do this if you use templating in the 'I' variable, but I agree that should be automatic!
I'll look into it!
Tks

I added the solution in the start thread.
I just don't know how to add commits.

Any luck with this? Having the same issue

I'll try to take a look at this tomorrow, let me see what I can do!

Any luck with this? Having the same issue

I'll try to take a look at this tomorrow, let me see what I can do!

Any luck with this? Having the same issue
You  can   make  it yourself -  just open   pidcontroller.py and  change after code after

# Calculate  terms
from
self._i_term += self._ki * error * delta_time
to
        if self._last_output == 0 :
           self._i_term += 0
        elif self._last_output == 100 :
           self._i_term += 0
        else:
           self._i_term += self._ki * error * delta_time

I'm no expert in coding, but after the last update, it seems that the readout of the integral-attribute is in saturation, but maybe not the regulator itself? I get the attribute
i: 2.3816e-16
I have the min/max to 0-100, windup to 100, and earlier the readout of the integral was the max-vaule, 100

The expected behavior is to the PID to increment the "I" until the output reaches the maximum. When on maximum the "I" will stop incrementing and remain constant. That is the expected. But should resume incrementing the "I" if the output falls bellow the maximum.


Inconsistent colour of icons in the app bar

Describe the bug
As seen below, the four icons on the right hand side have inconsistent colours. (Indeed, looking at the styles each individual one is trying to set a different one.)

Expected behavior
It is expected that all icons in the app bar have the same colour. Ideally all using the same style rather than each having their own.
Platform:

OpenEquella Version: 2018.2
OS: Debian
Browser Firefox

Additional context
Some discussion was had, and although at one point the notifications icon used a colour to indicate awaiting notifications, now that it has the badge it's agreed the colour can be uniform.

All done. Closing.


Windows not supported?

It's seems that packing the environment with empack does not work on windows. We should add some CI on Windows and check this.

this will a nobrainer once https://github.com/emscripten-forge/empack/pull/66 landed

This should be good now


Revert "[SP-4170] - Backport of PDI-16884 - Cassandra Output step doe…

…s not wo…"

This looks OK.


Add SSL configuration for Dashboards and other known issues

Description
[Describe what this change achieves]
Issues Resolved
[List any issues this PR will resolve]
Check List

[ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@Jon-AtAWS


Use bind host address instead of localhost

Fix node 18+ support for Frontend
Background
Solves #123
The mocks-server dependency does not appear to support IPV6 (as far as I can tell server.host documentation specifically mentions IPV4 addresses). You can see this in node 18+ by starting the mocks server and trying to reach http://[::1]:3100/api/apps/recently_updated the IPV6 localhost address. Swap the localhost address with something else that mocks-server can resolve.
Testing:
Using node 18 or newer start the dev server and make sure that the pages load correctly

See also https://github.com/mock-server/mockserver/issues/1051


Add support for Laravel 5.8.* and 6.* and PHP 7.3

I added the version numbers for 5.8.* and 6.* and modified the travis.yml file so that all possible combinations of PHP versions and Laravel are tested.
That would also fix #4.

Thank you!


F5 Command and F6 Command strings ignored on Ubuntu 18 (maybe others as well)

I just tried installing Atom+atom-python-run on a clean install of Ubuntu 18.04 and I'm experiencing a problem because Ubuntu 18.04 apparently does not symlink /usr/bin/python to /usr/bin/python3, hence there is no python command in PATH.
When I press F5 or F6 a new terminal window is opened with an error: There was an error creating the child process for this terminal Failed to execute child process "python" (No such file or dicrectory).
I tried writing python3 {file} in the [F5 Command] input field, but it appears to be ignored, because pressing F5 or F6 still cause the same error. I have tried with and without the checkmarks in [ ] Pause (F5) and [ ] Pause (F6).
See the two attached screenshots for error and settings.
I am not a daily Ubuntu user, I stumbled upon the issue testing various setups for teaching Python programming.
Edit: creating the symlink works though (ln -s /usr/bin/python3 /usr/bin/python).
Edit2: Atom v1.39.1 and atom-python-run v0.9.7

I have just tried what I tried above in Arch Linux (may daily driver) and Windows 10 (again for teaching testing purposes). In Windows 10 I can easily perform all the funny tricks with the F5/F6 Command command strings, e.g. notepad {file} and have atom-python-run open notepad with the file loaded, echo "whatever" to have atom-python-run print whatever etc.
I fail completely to do so on Arch Linux and Ubuntu 18. The command strings appear to be ignored. The same goes for the terminal string. Apart from the python vs python3 issue atom-python-run works out of the box with Ubuntu 18, but on Arch Linux I use another terminal (termite) but it does nothing for me to specify termite in the terminal settings string. It still reports the same error atom-python-run: TerminalError: There was a problem opening xterm - seemingly ignoring the settings string.

the previous comment worked for me, thanks. el comentario anterior funciono para mi, gracias.


DX: when displaying app version, don't put extra space if there is no CODENAME available

Coverage remained the same at 91.854% when pulling cc0a24f29bea628a8e3494f9c618a9a2b017e231 on keradus:2.18_codename into e1a260499fad38e15ec198556b471634e8f4c22f on FriendsOfPHP:2.18.


Fix example of hexadecimal sequence

Fix example of hexadecimal sequence
Fixes #5030

Please use the opposite ordering of lines in the commit message. Ideal example:
Fix example of hexadecimal sequence

Fixes #5030

The exact same goes for pull request title vs content. They should be swapped. Fix #5030 is not even clickable in the title, and you must open it to have any idea of what's going on.

@oprypin Ok, Thanks for the advice, Should I open a new PR ?

There's never a need to make a new PR. git commit --amend; git push -f can fix it.
But maybe you don't have to do anything because the person who clicks merge can edit the message, IIRC.

But maybe you don't have to do anything because the person who clicks merge can edit the message

So, is this ready to merge?


feat: unified interfaces improvements

Is there an existing issue for this?

[X] I have searched the existing issues

Is your feature request related to a problem? Please describe.
Currently the interfaces of methods are not unified.
Describe the solution you'd like.
No response
Describe an alternate solution.
No response
Anything else? (Additional Context)
No response

as a temp fix, created https://github.com/J-Quants/jquants-api-client-python/pull/26.


EditContributor list isn't filled

When the EditContributor screen is first opened, the contributor list isn't filled.

Wasn't set to Read Manually on test system


Adds --with-vagrant option

Motivation:
I am often building packages for an environment (debian8) that does not match my local box (a mac). Consequently, I generally add a virtual machine provisioner (vagrant) to my package.
It appears that the testing is failing due to a library not being available on the pandas build (ARM), which is unrelated to this specific change.

Coverage decreased (-0.4%) to 95.364% when pulling 806d614d50562b5668ca0b30cacd23660528d4b8 on brianbruggeman:master into 615198a2927cb5a913da8fd3196fa28d2d267abb on blue-yonder:master.

I still (even though I have now realized that this PR in fact would add an option to create a curated Vagrantfile) think that the feature is a bit too far out-of-scope. What PyScaffold does well is to integrate all those numerous tools of the Python ecosystem into a compelling default set, which would be challenging without PyScaffold. Adding a Vagrantfile to a repository is orthogonal to this problem. It is not especially difficult, vagrant init brings you very far already. I don't feel PyScaffold has a significant gain from such a feature, but once we have this flag in pyscaffold, we must support it indefinitely.

I'll close this PR now. Thank you for taking the time to put together a contribution, even though I cannot merge it. You are of course welcome to contribute to PyScaffold again. Discussing your implementation idea in an issue first, could help to avoid situations like this, where we do not feel comfortable merging a contribution.

Even if it is closed already, I would support that decision: The success of pyscaffold is for a good part based on the minimal approach. The less opinionated decisions it takes and the simpler it is (less options), the more people can and will use it. I love vagrant and I use it extensively, but the given template would have fitted probably only to a small subset of artifacts, furthermore not all packages are in a separated vagrant box. So I agree, it is kind of orthogonal to creating a minimal standard python package...
Maybe you can contribute your work on a "better starting point" for a vagrant file somewhere else, maybe vagrant itself, or even a small vagrantscaffold tool...thanks nonetheless for your contribution!


Spelling and cleanup...

+1

+1

Merged.  But doing a build puts the whitespace back into obj_dat.h  Hope to see another MR soon that will fix that issue.


Added readonly modifier to private fields that are never modified in files under mesh

@NewWheelTech  could update this repo to  .net Standard 2.0 or .net 8.0