Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra elipsis when displaying a single-event EventSet #410

Open
javiber opened this issue Mar 26, 2024 · 4 comments
Open

Extra elipsis when displaying a single-event EventSet #410

javiber opened this issue Mar 26, 2024 · 4 comments

Comments

@javiber
Copy link
Collaborator

javiber commented Mar 26, 2024

In this case the ellipsis (...) should not be displayed

Screenshot 2024-03-26 at 2 55 10 PM
@akshatvishu
Copy link
Contributor

I think this is happening due to :

            # Create ellipsis row between first half and last half if more than max_timestamps entries
            table.appendChild(html_table_row(dom, row))
            if (
                timestamp_idx == ((max_timestamps // 2) - 1)
                and num_timestamps > max_timestamps
            ) or (max_timestamps == 1):                                     # THIS IS CREATING THE ELLIPSE
                ellipsis_row = [ELLIPSIS] * (
                    1 + len(visible_feats) + int(has_hidden_feats)
                )
                table.appendChild(html_table_row(dom, ellipsis_row))

at here: def display_html.

Maybe we can do max_timestamps == 1 and num_timestamps > 1 instead of just or (max_timestamps == 1):

@javiber
Copy link
Collaborator Author

javiber commented Apr 2, 2024

Yes, the max_timestamps is the min(config.display_max_events, num_timestamps) so when we fixed the ellipsis for display_max_events == 1 we broke it when num_timestamps == 1
Adding that check would fix it, although I think we can factor-out the num_timestamps > max_timestamps and do something like:

(num_timestamps > max_timestamps) and ( (timestamp_idx == ((max_timestamps // 2) - 1)) or (max_timestamps == 1) )

and I feel that the last 2 conditions linked by the or could be simplified into a single one 🤔

@akshatvishu
Copy link
Contributor

akshatvishu commented Apr 2, 2024

Yes, the max_timestamps is the min(config.display_max_events, num_timestamps) so when we fixed the ellipsis for display_max_events == 1 we broke it when num_timestamps == 1 Adding that check would fix it, although I think we can factor-out the num_timestamps > max_timestamps and do something like:

(num_timestamps > max_timestamps) and ( (timestamp_idx == ((max_timestamps // 2) - 1)) or (max_timestamps == 1) )

and I feel that the last 2 conditions linked by the or could be simplified into a single one 🤔

Can you tell me how to run jupyter notebooks in our venv so, that I can test it because if I just download the jupyter extension from vscode and try to run something; the kernel just don't detect the poetry venv

@javiber
Copy link
Collaborator Author

javiber commented Apr 3, 2024

you can install jupyter bypassing the pyproject.toml with poetry run pip install jupyter and then poetry run jupyter notebook.
For vscode, I always set poetry config virtualenvs.path .venv and poetry config virtualenvs.in-project true so that poetry creates a folder .venv inside my project which vscode is able to find

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants