You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2024/12/17 16:45:25 UTC
[PR] Port Packed Producing Python Programs, #1008 [lucenenet]
paulirwin opened a new pull request, #1067:
URL: https://github.com/apache/lucenenet/pull/1067
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/main/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Ports the Python scripts for generating the various Lucene.Net.Util.Packed files.
Fixes #1008
## Description
This ports the four Python scripts in the Packed package/namespace that generate the various files there, and updates them to produce C# code instead, as well as updates them to be compatible with Python 3 (as Python 2, which was used at the time, is now very out of support). This results in some minor syntax differences in the generated code. The README.md file in this folder explains how to run the scripts (which is simply to run them with Python 3, no virtual environment or dependencies needed).
Having these scripts ported will help us ensure that the code is being generated the same way as in Java, as well as to assist any necessary future changes to how the files are generated. It also just helps complete our 1:1 port of the Lucene code, be it Java or Python.
This also corrects an out-of-date type name in the package.md documentation for this namespace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#issuecomment-2549026715
This diff is larger than I'd like it to be, so I'm going to update the scripts to deviate from Lucene's whitespace style to make the diff smaller and easier to review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1891130858
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
Good point about installing Python3 on Windows. Although, this is exactly what you need to go through to setup dotnet/runtime to build (along with several other dependencies), it is just a matter of having decent documentation for the developer setup process.
There are also other code generation scripts to port, some of which use Python.
That being said, I think we could probably skip all of that for the time being and change the CI builds (especially GitHub Actions) so they generate the files and then use Git to determine if there are any changes to them. If a generated file doesn't match the repository, fail the build. This would catch both cases:
1. The script changed and the files weren't regenerated and committed
2. One or more of the generated files was manually changed
If we do that, the normal build for developers wouldn't need to change and we wouldn't have to manually check to ensure things didn't get out of sync.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1894546938
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
Added GitHub Actions workflow to run the python scripts and check the git diff for any changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1893196642
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
Something else I just thought of, the upstream code has that `fixcrlf` task in the build.xml file, I need to determine if that's something we would need to do on Windows as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1891072246
##########
src/Lucene.Net/Util/Packed/gen_Packed64SingleBlock.py:
##########
@@ -0,0 +1,338 @@
+#! /usr/bin/env python
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# ## LUCENENET PORTING NOTES
+# This script was originally written for Python 2, but has been tested against Python 3.
+# No changes were necessary to run this script in Python 3.
+
+SUPPORTED_BITS_PER_VALUE = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 16, 21, 32]
+
+HEADER="""using Lucene.Net.Diagnostics;
+using Lucene.Net.Support;
+using System;
+using System.Runtime.CompilerServices;
+
+// This file has been automatically generated, DO NOT EDIT
Review Comment:
Maybe we should make this message more prominent and indicate how to generate the file so one doesn't have to search if they see the message.
```c#
//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by the gen_Packed64SingleBlock.py script.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
```
##########
src/Lucene.Net/Util/Packed/gen_PackedThreeBlocks.py:
##########
@@ -0,0 +1,192 @@
+#! /usr/bin/env python
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# ## LUCENENET PORTING NOTES
+# This script was originally written for Python 2, but has been tested against Python 3.
+# No changes were necessary to run this script in Python 3.
+
+HEADER="""using Lucene.Net.Diagnostics;
+using Lucene.Net.Support;
+using System;
+using System.Runtime.CompilerServices;
+
+// This file has been automatically generated, DO NOT EDIT
Review Comment:
Maybe we should make this message more prominent and indicate how to generate the file so one doesn't have to search if they see the message.
```c#
//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by the gen_PackedThreeBlocks.py script.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
```
##########
src/Lucene.Net/Util/Packed/gen_Direct.py:
##########
@@ -0,0 +1,224 @@
+#! /usr/bin/env python
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# ## LUCENENET PORTING NOTES
+# This script was originally written for Python 2, but has been tested against Python 3.
+# No changes were necessary to run this script in Python 3.
+
+# LUCENENET specific - we write the usings before the header
+HEADER="""// This file has been automatically generated, DO NOT EDIT
Review Comment:
Maybe we should make this message more prominent and indicate how to generate the file so one doesn't have to search if they see the message.
```c#
//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by the gen_Direct.py script.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
```
##########
src/Lucene.Net/Util/Packed/gen_BulkOperation.py:
##########
@@ -0,0 +1,414 @@
+#! /usr/bin/env python
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# ## LUCENENET PORTING NOTES
+# This script was originally written for Python 2, but has been modified to work with Python 3.
+# It is noted below where changes were made to support Python 3.
+# Some of the changes have been backported from newer versions of Lucene,
+# so when upgrading this script in the future, feel free to remove those comments.
+
+# LUCENENET backport - Python 3 support (lucene 945b1cb)
+try:
+ # python 3.9+
+ from math import gcd
+except ImportError:
+ # old python
+ from fractions import gcd
+
+"""Code generation for bulk operations"""
+
+MAX_SPECIALIZED_BITS_PER_VALUE = 24
+PACKED_64_SINGLE_BLOCK_BPV = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 16, 21, 32]
+OUTPUT_FILE = "BulkOperation.cs"
+
+# LUCENENET specific - we write the usings before the header, since
+# the numbered files don't need them
+HEADER = """// This file has been automatically generated, DO NOT EDIT
Review Comment:
Maybe we should make this message more prominent and indicate how to generate the file so one doesn't have to search if they see the message.
```c#
//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by the gen_BulkOperation.py script.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
```
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
While this documentation is useful to have, what is preventing us from adding these scripts to the build as was the case in Lucene?
https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/core/build.xml#L101-L127
Granted, we would need to add a Python3 dependency to Visual Studio, command line, and CI builds, but that doesn't seem like such a big deal. We would need a separate PR to update the main README so it can be applied after this is released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1891109211
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
Are you thinking to have the csproj execute the scripts before build? We could certainly do that, although given the low frequency with which these scripts change, and that the generated files are already in the build, I don't know if it's worth the extra effort for users to have to make sure they have Python set up first in order to contribute. This would likely impact Windows users the most, as it's more common for macOS and Linux users (particularly developers) to already have python3 in their PATH. And given that admittedly most .NET developers are using Windows, I worry that it's just a possible speed bump in getting new contributors on board.
What we could do is only do this in the build scripts and CI, so that anyone that just opens up the solution in VS/Rider doesn't have to have Python installed to build, but we still get the benefit of being able to test it locally in an automated way as well as have it run as part of the CI builds for regression testing of the scripts.
However we do it, note that there is already a PR for the beta18 documentation changes including README changes that could be updated: https://github.com/apache/lucenenet/pull/1043
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on code in PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#discussion_r1894545437
##########
src/Lucene.Net/Util/Packed/README.md:
##########
@@ -0,0 +1,28 @@
+# Lucene.Net.Util.Packed
+
+## Generating the files
+
+There are four Python scripts in this folder:
+[`gen_BulkOperation.py`](gen_BulkOperation.py),
+[`gen_Direct.py`](gen_Direct.py),
+[`gen_Packed64SingleBlock.py`](gen_Packed64SingleBlock.py), and
+[`gen_PackedThreeBlocks.py`](gen_PackedThreeBlocks.py).
+These scripts were ported from the original Lucene 4.8.1 code, and modified to generate the corresponding C# code instead of Java.
+
+To generate the files, run the following commands from this directory:
Review Comment:
FYI, this seems to work just fine on Windows. It checks out the files as crlf, and running the scripts again does not result in any pending changes, and they remain crlf after running the script.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Port Packed Producing Python Programs, #1008 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1067:
URL: https://github.com/apache/lucenenet/pull/1067#issuecomment-2558519066
I'm renaming the GitHub workflow file. I think we should keep the current names because it more closely matches the Java file names. This can always be revisited later in a separate PR if desired.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org