VBox is not working in 3.4.0rc1 about bokeh

[bryevdv]

This appears to be a bug in a validator method (referring to self.items)

bokeh/src/bokeh/models/layouts.py

Lines 456 to 460 in 8a5a35e

	@error(REPEATED_LAYOUT_CHILD)
	def _check_repeated_layout_children(self):
	children = [ item["child"] for item in self.items ]
	if len(children) != len(set(children)):
	return str(self)

@GillesFa can you see if changing to self.children in the code above fixes the issue for you? If so, a PR would be appreciated.

[GillesFa]

@bryevdv yes indeed it fixes it. Looks like HBox has the same issue.
thanks for your support.

I am quite new in github so not sure how to create the PR.

[mosc9575]

@GillesFa If you want we can guide you through the PR process. Please read this tutorial for making a successful pull request.

After you created a new branch, you can change the two lines in ~/bokeh/src/bokeh/models/layouts.py.
It would also be great to add a test in ~tests/unit/bokeh/models/test_layouts__models.py.

The test could look like below:

from bokeh.models.layouts import Row, Column, HBox, VBox, LayoutDOM, TabPanel # isort:skip

def check_children_prop_boxes(layout_callable: type[HBox | VBox]):
    ## component subclasses are layouts, widgets and plots
    components = [Row(), Column(), figure()]

    # Test layout accepts children argument
    layout2 = layout_callable(children=components)
    assert layout2.children == [{'child': c} for c in components]
    assert layout2._check_repeated_layout_children() == []

    # Test value error raised when non-layout is provided as children
    with pytest.raises(ValueError):
        layout_callable(children=[ColumnDataSource()])

def test_VBox() -> None:
    check_props_with_sizing_mode(VBox())
    check_children_prop_boxes(VBox)

def test_HBox() -> None:
    check_props_with_sizing_mode(HBox())
    check_children_prop_boxes(HBox)

See also the guides for testing and writing tests.

[GillesFa]

working on it

[GillesFa]

Hi All, I am trying to push my changes but it fails: E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 37 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 38 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 39 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 40 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 41 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 42 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 43 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 44 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 45 E File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 46 E assert 353802 == 0 E + where 353802 = len(['File contains carriage returns at end of line: .github/CONTRIBUTING.md, line 1', 'File contains carriage returns at ...e: .github/CONTRIBUTING.md, line 5', 'File contains carriage returns at end of line: .github/FUNDING.yml, line 1', ...]) tests\codebase\test_code_quality.py:48: AssertionError =========================== short test summary info =========================== FAILED tests/codebase/test_code_quality.py::test_code_quality - AssertionErro... ============================= 1 failed in 27.98s ============================== Run ESLint...............................................................Passed Run isort checks.........................................................Passed Run ruff checks..........................................................Passed Run vermin checks........................................................Passed Validate LICENSE file....................................................Passed Verify package exclusions................................................Passed Verify that OO mode works................................................Passed Check for reserved windows filenames.....................................Passed Protect Git branches.....................................................Passed error: failed to push some refs to 'https://github.com/GillesFa/bokeh.git' (bkdev) C:\Development\bokeh\bokehjs> Is it failing because of the carriage returns? Gilles Faure | Senior Software Engineer Rio Tinto Centre for Mine Automation Australian Centre for Field Robotics Faculty of Engineering | The University of Sydney Rm 303, Sydney Robotics Hub J18 8 Little Queen St, Chippendale NSW 2008 From: Moritz Schreiber ***@***.***> Sent: Thursday, March 7, 2024 10:14 PM To: bokeh/bokeh ***@***.***> Cc: Gilles Faure ***@***.***>; Mention ***@***.***> Subject: Re: [bokeh/bokeh] VBox is not working in 3.4.0rc1 (Issue #13743) @GillesFa<https://github.com/GillesFa> If you want we can guide you through the PR process. Please read this tutorial for making a successful pull request<https://docs.bokeh.org/en/latest/docs/dev_guide/pull_requests.html>. After you created a new branch, you can change the two lines in ~/bokeh/src/bokeh/models/layouts.py. It would also be great to add a test in ~tests/unit/bokeh/models/test_layouts__models.py. The test could look like below: from bokeh.models.layouts import Row, Column, HBox, VBox, LayoutDOM, TabPanel # isort:skip def check_children_prop_boxes(layout_callable: type[HBox | VBox]): ## component subclasses are layouts, widgets and plots components = [Row(), Column(), figure()] # Test layout accepts children argument layout2 = layout_callable(children=components) assert layout2.children == [{'child': c} for c in components] assert layout2._check_repeated_layout_children() == [] # Test value error raised when non-layout is provided as children with pytest.raises(ValueError): layout_callable(children=[ColumnDataSource()]) def test_VBox() -> None: check_props_with_sizing_mode(VBox()) check_children_prop_boxes(VBox) def test_HBox() -> None: check_props_with_sizing_mode(HBox()) check_children_prop_boxes(HBox) — Reply to this email directly, view it on GitHub<#13743 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AZXIO2RXZ77ALYJFTR2G4YTYXBDYJAVCNFSM6AAAAABEKDBE52VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBTGI4DQNRZGI>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

[mosc9575]

The error

bokeh/tests/codebase/test_code_quality.py

Line 62 in c8fdc57

message_carriage = "File contains carriage returns at end of line: {path}, line {line_no}"

is raised

bokeh/tests/codebase/test_code_quality.py

Lines 107 to 108 in c8fdc57

	if line.endswith("\r\n") or line.endswith("\r"):
	errors.append((message_carriage, fname, line_no))

It looks like you have to change all line endings in your local bokeh repository. This is usually just a setting in your IDE.

[bryevdv]

@GillesFa For context, there are different conventions around line-endings on different platforms (e.g. Windows vs Linux) and for consistency, we enforce a single convention in the entire codebase. If you let use know what editor or IDE you are using I can try to find specific guidance about changing the setting for line-endings.

[GillesFa]

@mosc9575 and @bryevdv,
It makes sense to have this single convention. I opened only 2 files (bokeh\src\bokeh\models\layouts.py and bokeh\tests\unit\bokeh\models\test_layouts__models.py) so I would understand that the error happens for them. But it happens to almost every files! The other app that accessed all the files is git (git version 2.42.0.windows.2) but I would not expect git to change the files!

I used notepad++ to open the 2 files above. I changed my setting to use unix(LF) EOL but I still get the errors.
I changed the EOL of another file (../tests/unit/bokeh/models/test_grids.py), git sees it as different file:

(bkdev) C:\Development\bokeh\bokehjs>git status
On branch 13743_VBox-is-not-working-in-3.4.0rc1
Your branch is ahead of 'origin/13743_VBox-is-not-working-in-3.4.0rc1' by 1 commit.
(use "git push" to publish your local commits)

Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: ../src/bokeh/models/layouts.py
modified: ../tests/unit/bokeh/models/test_grids.py
modified: ../tests/unit/bokeh/models/test_layouts__models.py

no changes added to commit (use "git add" and/or "git commit -a")

This is what I get when showing the diff:

(bkdev) C:\Development\bokeh\bokehjs>git diff ../tests/unit/bokeh/models/test_grids.py
warning: in the working copy of 'tests/unit/bokeh/models/test_grids.py', LF will be replaced by CRLF the next time Git touches it

I changed the EOL of the file to windows(CRLF) and git does not show the file as changed anymore. So it looks like my local repo as the windows EOL. But I do not understand why because they were only downloaded from the git server and never loaded to an IDE!

Not sure what is going on there!

[mosc9575]

@GillesFa Here is a complete guide about line endings in git.

Edit:
I was doing some research and I want to share this really good SO post with you. To me it looks like that autocrlf=true or autocrlf=auto is doing it's job on your windows system. The problem is, that bokeh checks the line endings in a push and not in a commit process.
You could try to run git-add --renormalize . after changing core.eol to lf for your repo.

Hope this helps and brings you a step closer to a pull request.

[GillesFa]

I ended up re-cloning the repo. I looks like it works this time.
I created the PR #13757.

The windows and MAC BokehJs -CI fail but pass for Ubuntu! Not sure why is it.
Can you tell me the best way to investigate it?
thanks

[GillesFa]

Hi guys,
not sure what is going on with my environment but I get the above errors again (File contains carriage returns at end of line: tests/unit/bokeh/util/test_warnings.py, line 37).
My git config seems to be ok:

(bkdev) C:\Development\bokeh>git config --list
diff.astextplain.textconv=astextplain
filter.lfs.clean=git-lfs clean -- %f
filter.lfs.smudge=git-lfs smudge -- %f
filter.lfs.process=git-lfs filter-process
filter.lfs.required=true
http.sslbackend=openssl
core.autocrlf=true
core.fscache=true
core.symlinks=false
pull.rebase=false
credential.helper=manager
credential.https://dev.azure.com.usehttppath=true
init.defaultbranch=master
core.repositoryformatversion=0
core.filemode=false
core.bare=false
core.logallrefupdates=true
core.symlinks=false
core.ignorecase=true
remote.origin.url=https://github.com/GillesFa/bokeh.git
remote.origin.fetch=+refs/heads/:refs/remotes/origin/
branch.branch-3.4.remote=origin
branch.branch-3.4.merge=refs/heads/branch-3.4
remote.upstream.url=https://github.com/bokeh/bokeh.git
remote.upstream.fetch=+refs/heads/:refs/remotes/upstream/
branch.13743_VBox-is-not-working-in-3.4.0rc1.remote=origin
branch.13743_VBox-is-not-working-in-3.4.0rc1.merge=refs/heads/13743_VBox-is-not-working-in-3.4.0rc1

I had a look to this blog https://www.aleksandrhovhannisyan.com/blog/crlf-vs-lf-normalizing-line-endings-in-git/.
If I run the cmd "git ls-files tests/unit/bokeh/util/test_warnings.py --eol" then I get this:
i/lf w/crlf attr/ tests/unit/bokeh/util/test_warnings.py
It looks like my environment is setup correctly.

Is test_code_quality.py checking the working enviroment? if so then crlf will be found but if I understand the blog above then git will change the crlf into lf when pushing the code.

I tried to setup a Ubuntu env but I have got issue here!
### Issue 1:
when I run:

(bkdev) gillesfaure@abacus:~/bokeh$ cd bokehjs
(bkdev) gillesfaure@abacus:~/bokeh/bokehjs$ npm install --location=global npm

then I get this:

removed 52 packages, and changed 71 packages in 1s

24 packages are looking for funding
  run `npm fund` for details

This is what I get when running "npm fund":

(bkdev) gillesfaure@abacus:~/bokeh/bokehjs$ npm fund
@bokeh/[email protected]

### Issue 2:
When I run this command:
(bkdev) gillesfaure@abacus:~/bokeh$ python scripts/hooks/install.py
I get this:

pre-commit installed at .git/hooks/pre-push
[INFO] Initializing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Initializing environment for https://github.com/abravalheri/validate-pyproject.
[INFO] Initializing environment for https://github.com/abravalheri/validate-pyproject:.[all].
[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Initializing environment for local.
[INFO] Installing environment for https://github.com/astral-sh/ruff-pre-commit.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/home/gillesfaure/miniconda3/envs/bkdev/bin/python3.1', '-mvirtualenv', '/home/gillesfaure/.cache/pre-commit/repo2ld_5990/py_env-python3.1', '-p', 'python3.1')
return code: 1
stdout:
    RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.1'
stderr: (none)
Check the log at /home/gillesfaure/.cache/pre-commit/pre-commit.log

See attached log for more details.

At this stage I am unable to push my changes, Any idea how we can solve it?

thanks
Gilles

pre-commit.log

[mosc9575]

@GillesFa

Let's first talk about line ending on windows and how to force LF. It looks like my earlier comment was confusing and not clear.

There are two git config attributes that affect the line endings: core.autocrlf and core.eol.
Previously, you were told to use core.autocrlf = true to be able to work on cross-platform projects, but it's not true any more.

If your system/IDE/tooling support LF and you do want to use LF as everyone else in your team without any silent lf->crlf->lf normalization, you must turn off autocrlf and configure eol to not infer native line endings, but force it to use lf.

Use this lines to set the configs.

git config core.eol lf
git config core.autocrlf false

With your setting core.autocrlf = true the line endings are forced to the windows standard, which will raise the known error on a push.

---

The second issue is just an information and not a warning.

removed 52 packages, and changed 71 packages in 1s

24 packages are looking for funding
  run `npm fund` for details

This means, that some projects are looking for monetary support.

---

The last issue is a problem with a trailing zero after a dot.

In your shared log your python executable is python3.1

sys.executable: /home/gillesfaure/miniconda3/envs/bkdev/bin/python3.1

but most likely you are using python 3.10. Please check if you can update and correct your python_spec='python3.1'.

[bryevdv]

I actually looks like the env is 3.11:

    3.11.8 | packaged by conda-forge | (main, Feb 16 2024, 20:53:32) [GCC 12.3.0]

I'm not sure why the pre-commit hook is mistakenly getting "3.1" maybe something has changed that we will need to update or account for, though I don't see anything that looks amiss in our .pre-commit-config.yaml file 🤷 @GillesFa what exact commands did you use to create and activate the environment? I will try to reproduce on a VM (or perhaps @tcmetzger can try if he as windows more readily available)

[GillesFa]

The pre-commit hook issue is happeing on Ubuntu 20.04.6 LTS (GNU/Linux 5.4.0-167-generic x86_64) and RHEL7x64.
It is working fine for windows.

This is the steps I use to set up my Ubuntu and RedHat env:

git clone https://github.com/GillesFa/bokeh.git
cd bokeh
git remote add upstream https://github.com/bokeh/bokeh.git
git fetch upstream
git checkout 13743_VBox-is-not-working-in-3.4.0rc1
conda update -n base -c defaults conda
conda env create -n bkdev -f conda/environment-test-3.11.yml
conda activate bkdev
cd bokehjs
npm install --location=global npm
npm ci
cd ..
python scripts/hooks/install.py