Skip to content

Add visible property to table columns#11501

Merged
bryevdv merged 4 commits intobranch-2.4from
bryanv/11423_table_column_visible
Aug 18, 2021
Merged

Add visible property to table columns#11501
bryevdv merged 4 commits intobranch-2.4from
bryanv/11423_table_column_visible

Conversation

@bryevdv
Copy link
Member

@bryevdv bryevdv commented Aug 12, 2021 •
edited
Loading

@bryevdv bryevdv added this to the 2.4 milestone Aug 12, 2021
@bryevdv bryevdv changed the title checkpoint Add visible property to table columns Aug 12, 2021
@bryevdv bryevdv force-pushed the bryanv/11423_table_column_visible branch 3 times, most recently from 53551e8 to c208853 Compare August 13, 2021 01:27
@bryevdv
Copy link
Member Author

bryevdv commented Aug 13, 2021 •
edited
Loading

@mattpap Do you have any suggestions for testing this?

Edit: Also whether column.change is too general for a layout invalidation.

@bryevdv bryevdv requested a review from mattpap August 13, 2021 20:39
@mattpap
Copy link
Contributor

mattpap commented Aug 14, 2021

Like this:

bokeh/bokehjs/test/integration/regressions.ts

Lines 48 to 61 in ac69e52

it("disallows to change FactorRange to a lower dimension with a different number of factors", async () => {
const p = fig([200, 200], {
title: null,
toolbar_location: null,
x_range: new FactorRange({factors: [["a", "b"], ["b", "c"]]}),
y_range: new DataRange1d(),
})
const source = new ColumnDataSource({data: {x: [["a", "b"], ["b", "c"]], y: [1, 2]}})
p.vbar({x: {field: "x"}, top: {field: "y"}, width: 0.1, source})
const {view} = await display(p)
source.data = {x: ["a"], y: [1]}
;(p.x_range as FactorRange).factors = ["a"]
await view.ready

i.e. create a table, await display, modify visibility and await ready; just under widgets, not regressions.

@bryevdv bryevdv force-pushed the bryanv/11423_table_column_visible branch from 53ae77f to 94620a8 Compare August 18, 2021 03:21
@bryevdv
Copy link
Member Author

bryevdv commented Aug 18, 2021

Just reiterating that the current process for adding the necessary artifact is quite fraught. This branch was only a ~week old but there were new tests on branch-2.4 that showed up in the downloaded report. Just unzipping the report added the artifacts for those newer tests that showed up as new files to add. A rebase failed while the files existed, and deleting them is difficult due to the special characters in the filenames. Eventually resorted to fit clean -xfd in the tests dir to be able to rebase and re-unzip. I can figure all this out but I think it is a very high bar for ay new contributor. cc @tcmetzger for visibility in to these possibilities.

@mattpap
Copy link
Contributor

mattpap commented Aug 18, 2021

Given that effectively you're not testing this branch, but a merge, then appearance of files from that merge shouldn't be surprising. In fact it can go much worse that that. A week old PR is actually quite old given there were 19 PRs merged during this timespan. I rebase my PRs pretty much every time I push (assuming there are any changes to pull in). This not only helps me to avoid major conflicts (especially after PRs like #11503), but also makes sure that I'm fully aware what I'm testing. I doubt there is much we can do to improve this, as long as we test merges, that wouldn't be prone to other issues.

@mattpap
Copy link
Contributor

mattpap commented Aug 18, 2021

(...) deleting them is difficult due to the special characters in the filenames (...)

Which exactly? Troublesome (to a shell) characters are encoded (or dropped) to make it easier to operate on baseline files.

mattpap
mattpap reviewed Aug 18, 2021
View reviewed changes
bryevdv
bryevdv commented Aug 18, 2021
View reviewed changes
@bryevdv bryevdv merged commit a57ea2b into branch-2.4 Aug 18, 2021
@bryevdv bryevdv deleted the bryanv/11423_table_column_visible branch August 18, 2021 15:51
@charlesbluca charlesbluca mentioned this pull request Aug 18, 2021
Open
2 tasks
bryevdv added a commit that referenced this pull request Dec 13, 2021
@bryevdv
Add visible property to table columns (#11501)
3d6fc10 
* Add visible property to Table columns

* add test

* add test artifacts

* Update examples/reference/models/data_table_server.py
@github-actions
Copy link

github-actions bot commented Oct 26, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mattpap mattpap mattpap left review comments

Assignees

No one assigned

Labels

status: accepted

Projects

None yet

Milestone

2.4

Development

Successfully merging this pull request may close these issues.

[FEATURE] visible property for TableColumns

2 participants

@bryevdv @mattpap