Hines/fix 8.2.2 #2486

nrnhines · 2023-09-02T17:56:58Z

Cherry picks (and fixes resulting conflicts)
138282b
f838453
4eaf27f

The principal reason is to fix the cursor backspace issue for a prospective 8.2.3.
I believe this also fixes the windows cursor backspace issue by consequence of a build with a more recent libreadline8.dll

…inal. (#2258) * MINGW: if in yyparse, rl_getc_function should = rl_getc * Mac: rl_getc_function is always getc_hook. Allow multiline statements. Note: if, in a multline statement, the gui tries to execute a statement, an error is generated: hoc_run1: caught exception: hoc_execerror: Cannot reenter parser. Maybe you were in the middle of a direct command. * test using nrniv -isatty Mac and Windows have trouble with the test.

* Change MacOS wheel building with ncurses 6.4 as well * Secure file on AzureCI is updated * Update the documentation for better clarity Co-authored-by: Pramod S Kumbhar <pramod.s.kumbhar@gmail.com>

codecov · 2023-09-02T18:13:23Z

Codecov Report

Merging #2486 (90a2029) into release/8.2 (93d41fa) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           release/8.2    #2486      +/-   ##
===============================================
- Coverage        46.45%   46.45%   -0.01%     
===============================================
  Files              526      526              
  Lines           119261   119260       -1     
===============================================
- Hits             55404    55403       -1     
  Misses           63857    63857

Files Changed	Coverage Δ
src/oc/hoc.cpp	`68.05% <ø> (ø)`

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

alexsavulescu · 2023-09-02T19:34:07Z

Some(several?) updates must be cherry picked into release/8.2 , i.e. #2433

Cherry picked from 116a541.

nrnhines · 2023-09-02T21:28:50Z

Some(several?) updates must be cherry picked into release/8.2 , i.e. #2433

My git cherry-pick workflow does not seem well founded for this project. And there are 30 py files that black changes.

alexsavulescu · 2023-09-03T11:49:48Z

Some(several?) updates must be cherry picked into release/8.2 , i.e. #2433

My git cherry-pick workflow does not seem well founded for this project. And there are 30 py files that black changes.

I usually create a branch from release/8.2 and cherry-pick the commits from master that are related to the CI. But yeah, a bit of manual effort is in order

pramodk · 2023-09-04T08:06:28Z

@nrnhines : did you check the master and see if there are other commits that we need to back port?

nrnhines · 2023-09-04T11:42:59Z

did you check the master

Yes. This PR has the substantive user fixes. Of course I'm willing to entertain others I might have missed. But I'm having trouble with the minimal build and CI related changes needed for 8.2.3 to pass and build the wheels and setup.exe.
d8c113a above had 5 conflicts. I fear that one setup.py to rule them all (#2235) needs to be cherry picked first but it has many conflicts as well. I worry that a few things about it are silently accepted and if I choose to resolve a conflict in favor of old 8.2.2, I'll introduce a bug. I am playing with a temporary PR #2487 that focuses just on the build issue but can't yet succeed with a windows build.

ramcdougal · 2023-09-07T13:58:40Z

And there are 30 py files that black changes.

Out of curiosity, does anyone understand how this can be? black was run on everything, and no new changes are allowed unless they're unchanged by running black

azure-pipelines · 2023-09-07T19:29:07Z

✔️ 2e86f71 -> Azure artifacts URL

- all changes look straightforward

azure-pipelines · 2023-09-07T21:50:29Z

✔️ 7fb42f5 -> Azure artifacts URL

nrnhines · 2023-09-07T23:35:52Z

@pramodk

Disable shallow clone with the option

What was the rationale for that?

pramodk · 2023-09-08T08:50:28Z

What was the rationale for that?

I didn't spend much time but the CI [failure here was](https://github.com/neuronsimulator/nrn/actions/runs/6114834861/job/16597216261
https://tracking.duperrex.ch/2p8refhd):

fatal: No names found, cannot describe anything.

Hence tried to do a full clone. (Didn't check if there is any other change in the config somewhere in current master. cc: @heerener)

nrnhines · 2023-09-10T19:16:11Z

@ramcdougal

And there are 30 py files that black changes.

Out of curiosity, does anyone understand how this can be? black was run on everything, and no new changes are allowed unless they're unchanged by running black

I don't have a story yet, but here are some observations.

black share/lib/python/neuron/doc.py exhibits minor reformatting for both master and 8.2.2

8.2.2 .github/workflows/formatting.yml runs black on all respository py,ipynb files with

        run: |
          git ls-tree -z -r HEAD --name-only \
          | grep -z '\.\(py\|ipynb\)$' \
          | xargs -0 black --check --color --diff --target-version py37

whereas the master version of this file uses

run: external/coding-conventions/bin/format -v --dry-run

Running the latter manually does not change any files in master (coding-conventions HEAD detached at a377933) and on examining stdout/stderr, the black line that mentions doc.py

external/coding-conventions/bin/format -v --dry-run >& temp
...
/home/hines/neuron/temp/.bbp-project-venv/bin/black --color --target-version py37 --check --diff --color share/lib/python/neuron/psection.py ...  share/lib/python/neuron/doc.py ...
All done!
30 files would be left unchanged.

Just for fun, I tried running the last also in 8.2.2 (note that coding-conventions for master is a half dozen commits ahead of what
8.2.2 gets (5d4bcd2d) and I had to $ cp external/coding-conventions/.bbp-project.yaml .)
There were many errors about clang-format, but

/home/hines/neuron/8.2.3/.bbp-project-venv/bin/black ... share/lib/python/neuron/doc.py ...
error: cannot format share/lib/python/neuron/rxd/geometry3d/graphicsPrimitives.pyx: Cannot parse: 9:8: cimport cython
Oh no!
29 files would be left unchanged, 1 file would fail to reformat.

And at the end there are no modified files.

I wonder if, in fact, the master CI black check never fails.

ramcdougal · 2023-09-10T20:03:03Z

It used to fail, because I've had to go back and run black before.

I wonder if it's possible that different versions of black do different things.

pramodk · 2023-09-12T14:57:37Z

@nrnhines : I have asked internally if someone is aware about black issue.

About the documentation failure, I see:

In #2379, Alex has already documented the following in this file:

## Hacking the Python Domain

 It is sometimes required to re-hack the domain to make it work with the latest version of Sphinx. 
 To that end, the following script can be used to generate the domain from the Python domain:

 ```bash
 cd docs
 python3 generate_hocdomain.py
 

 This script generates a HOC domain from the one available in the sphinx package and writes it to:

     docs/domains/hocdomain.py

 A comment is added at the top of the file to indicate that it is a generated file and the Sphinx version used to generate it.

I see Running Sphinx v7.2.5 in documentation CI. So let me take docs/domains/hocdomain.py from master and see what happens.

…lity

heerener · 2023-09-12T15:35:57Z

I'd say this is a matter of black evolving and getting better. As an example: in the python formatting run here we're using black 23.3.0
In the changelog for version 23.1.0, I find this item:

Remove unnecessary parentheses from tuple unpacking in for loops (#2945) (22.3.0)

(see the related black issue#2338 for an example)
That looks suspiciously like the last item black suggests in the failed workflow run here.

23.1.0 was the first version of 2023, released after what, to me, looks like the last successful run of this workflow on release/8.2: #2113 (15th of December 2022, before this particular version of black was released).

pramodk · 2023-09-12T21:17:17Z

Reopening to re-trigger failing windows CI!

azure-pipelines · 2023-09-12T21:50:22Z

✔️ 22782f6 -> Azure artifacts URL

azure-pipelines · 2023-09-13T09:33:30Z

✔️ 7f811c6 -> Azure artifacts URL

pramodk · 2023-09-13T10:04:28Z

thanks to @heerener's hints, we verified that the master doesn't fail because we install black version ~22.3

nrn/.bbp-project.yaml

Line 8 in 05deaaf

version: '[jupyter] ~=22.3'

So one of the previous commits I downgraded the installed version. So now CI seems green (expect code coverage which should be ignored here).

nrnhines · 2023-09-13T11:08:41Z

now CI seems green

Excellent! When I got up this morning, I was ready to go ahead and pollute this PR with the 30 python files modified by black.
But I think this black downgrade is a better solution. Now just need an approving review :) (and all the details for tag and user facing info and urls for the installers)

pramodk · 2023-09-13T12:25:15Z

I was ready to go ahead and pollute this PR with the 30 python files modified by black. But I think this black downgrade is a better solution.

Also discussed this with @heerener. I think in coming days we can change python files in master with newer Black and then backport those to 8.2 branch.

Now just need an approving review

Will do!

and all the details for tag and user facing info and urls for the installers

Before tag, we also need to update changelog updated in docs/changelog.md. Do you want to do this in this PR or a separate PR?

nrnhines · 2023-09-13T12:28:58Z

want to do this in this PR

I'll merge this. And start working on the changelog. Thanks!

pramodk

LGTM. Just as a reminder - this won't be a squash and merge :)

nrnhines · 2023-09-13T12:37:26Z

this won't be a squash and merge

I don't understand. I was going to select squash and merge for this (and edit the commit message). Another PR will update the changelog.md . I am a bit shaky about the chicken/egg problem for updating the https://nrn.readthedocs.io/en/8.2.2/
Seemed reasonable to me to separate the substantive PR from the administrative PR.

Or is this related to

Required statuses must pass before merging

nrnhines · 2023-09-13T15:04:40Z

Final CI Expected — Waiting for status to be reported

I don't know how to deal with this. Hopefully, dealing with it does not require starting CI from scratch.

alexsavulescu · 2023-09-13T18:54:06Z

Final CI Expected — Waiting for status to be reported

I don't know how to deal with this. Hopefully, dealing with it does not require starting CI from scratch.

I believe this needs to go in: https://github.com/neuronsimulator/nrn/pull/2332/files

alexsavulescu · 2023-09-13T18:58:44Z

The final CI was added for convenience, otherwise for the required status one would have to manually select the jobs from the NEURON CI matrix; any change in the options of that matrix (i.e. CI job name change, adding a CMake option, ..) would require a significant manual labor to change the required statuses. Now instead of requiring that all those jobs individually, the FInal CI comes into play.

Fixes "Final CI Expected --- Waiting for status to be reported"

azure-pipelines · 2023-09-13T20:47:29Z

✔️ 90a2029 -> Azure artifacts URL

nrnhines and others added 3 commits September 2, 2023 13:05

Windows 11 fix for hoc icon. (#2279)

17de17c

Fixes linux wheel backspace by upgrading ncurses-6.2 to 6.4 (#2484)

3627da0

* Change MacOS wheel building with ncurses 6.4 as well * Secure file on AzureCI is updated * Update the documentation for better clarity Co-authored-by: Pramod S Kumbhar <pramod.s.kumbhar@gmail.com>

Pin cython to < 3. Workaround for #2430 (#2433)

d8c113a

Cherry picked from 116a541.

nrnhines added 2 commits September 7, 2023 07:56

Eliminate non cython<3 changes from cherrypick

32b69d9

Bump VERSION to 8.2.3 in CMakeLists.txt

9707a74

nrnhines added 4 commits September 7, 2023 11:10

Remove azure build of GPU wheels

7aaaea1

more gpu wheel deletion

b48faee

Use <cstdint>

c05f4bc

deal with .inputrc if missing on windows

2e86f71

pramodk added 2 commits September 7, 2023 23:12

Backport changes from github docs workflow

eeb08cd

- all changes look straightforward

Disable shallow clone with the option

7fb42f5

use docs/domains/hocdomain.py from master for latest sphinx compatibi…

22782f6

…lity

pramodk closed this Sep 12, 2023

pramodk reopened this Sep 12, 2023

downgrade black version

7f811c6

pramodk force-pushed the hines/fix-8.2.2 branch from 693cab2 to 7f811c6 Compare September 13, 2023 08:29

pramodk approved these changes Sep 13, 2023

View reviewed changes

Picked the "final:" fragment out of the master version.

90a2029

Fixes "Final CI Expected --- Waiting for status to be reported"

nrnhines merged commit 0a0dd37 into release/8.2 Sep 13, 2023
35 of 36 checks passed

nrnhines deleted the hines/fix-8.2.2 branch September 13, 2023 21:33

heerener mentioned this pull request Sep 20, 2023

NEURON 8.2.3 patch release #2505

Closed

22 tasks

nrnhines mentioned this pull request Jun 25, 2024

NEURON 8.2.6 patch release #2930

Closed

27 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hines/fix 8.2.2 #2486

Hines/fix 8.2.2 #2486

nrnhines commented Sep 2, 2023

codecov bot commented Sep 2, 2023 •

edited

Loading

alexsavulescu commented Sep 2, 2023

nrnhines commented Sep 2, 2023 •

edited

Loading

alexsavulescu commented Sep 3, 2023

pramodk commented Sep 4, 2023

nrnhines commented Sep 4, 2023

ramcdougal commented Sep 7, 2023

azure-pipelines bot commented Sep 7, 2023

azure-pipelines bot commented Sep 7, 2023

nrnhines commented Sep 7, 2023

pramodk commented Sep 8, 2023

nrnhines commented Sep 10, 2023

ramcdougal commented Sep 10, 2023

pramodk commented Sep 12, 2023

heerener commented Sep 12, 2023

pramodk commented Sep 12, 2023

azure-pipelines bot commented Sep 12, 2023

azure-pipelines bot commented Sep 13, 2023

pramodk commented Sep 13, 2023

nrnhines commented Sep 13, 2023

pramodk commented Sep 13, 2023

nrnhines commented Sep 13, 2023

pramodk left a comment

nrnhines commented Sep 13, 2023 •

edited

Loading

nrnhines commented Sep 13, 2023

alexsavulescu commented Sep 13, 2023

alexsavulescu commented Sep 13, 2023

azure-pipelines bot commented Sep 13, 2023

Hines/fix 8.2.2 #2486

Hines/fix 8.2.2 #2486

Conversation

nrnhines commented Sep 2, 2023

codecov bot commented Sep 2, 2023 • edited Loading

Codecov Report

alexsavulescu commented Sep 2, 2023

nrnhines commented Sep 2, 2023 • edited Loading

alexsavulescu commented Sep 3, 2023

pramodk commented Sep 4, 2023

nrnhines commented Sep 4, 2023

ramcdougal commented Sep 7, 2023

azure-pipelines bot commented Sep 7, 2023

azure-pipelines bot commented Sep 7, 2023

nrnhines commented Sep 7, 2023

pramodk commented Sep 8, 2023

nrnhines commented Sep 10, 2023

ramcdougal commented Sep 10, 2023

pramodk commented Sep 12, 2023

heerener commented Sep 12, 2023

pramodk commented Sep 12, 2023

azure-pipelines bot commented Sep 12, 2023

azure-pipelines bot commented Sep 13, 2023

pramodk commented Sep 13, 2023

nrnhines commented Sep 13, 2023

pramodk commented Sep 13, 2023

nrnhines commented Sep 13, 2023

pramodk left a comment

Choose a reason for hiding this comment

nrnhines commented Sep 13, 2023 • edited Loading

nrnhines commented Sep 13, 2023

alexsavulescu commented Sep 13, 2023

alexsavulescu commented Sep 13, 2023

azure-pipelines bot commented Sep 13, 2023

codecov bot commented Sep 2, 2023 •

edited

Loading

nrnhines commented Sep 2, 2023 •

edited

Loading

nrnhines commented Sep 13, 2023 •

edited

Loading