Skip to content

ENH: Add possibility to turn section invisible#288

Merged
merveenoyan merged 2 commits intoskops-dev:mainfrom
BenjaminBossan:ENH-possibility-to-make-section-invisible
Feb 7, 2023
Merged

ENH: Add possibility to turn section invisible#288
merveenoyan merged 2 commits intoskops-dev:mainfrom
BenjaminBossan:ENH-possibility-to-make-section-invisible

Conversation

@BenjaminBossan
Copy link
Copy Markdown
Collaborator

Sections of model cards have an additional flag called visible. It is by default True but if set to False, the corresponding section, and its subsections, are not rendered.

In contrast to deleting those section, turning them invisible will not remove them from the underlying data dict. This is useful because it allows us to restore those sections to exactly their previous place. In contrast, if we delete and add them again, they may end up in a different position, because the order of the underlying dict can change.

At the moment, the feature is not used anywhere in skops. I plan, however, to use it for a model card creation space, where it would be quite useful to have.

Sections of model cards have an additional flag called visible. It is by
default True but if set to False, the corresponding section, and its
subsections, are not rendered.

In contrast to deleting those section, turning them invisible will not
remove them from the underlying data dict. This is useful because it
allows us to restore those sections to exactly their previous place. In
contrast, if we delete and add them again, they may end up in a
different position, because the order of the underlying dict can change.

At the moment, the feature is not used anywhere in skops. I plan,
however, to use it for a model card creation space, where it would be
quite useful to have.
@BenjaminBossan
Copy link
Copy Markdown
Collaborator Author

@skops-dev/maintainers ready for review

Copy link
Copy Markdown
Collaborator

@merveenoyan merveenoyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can merge, this looks like a small and straightforward change. Would be nice to document later on after the Space is out.

@merveenoyan merveenoyan merged commit f9e2668 into skops-dev:main Feb 7, 2023
@BenjaminBossan BenjaminBossan deleted the ENH-possibility-to-make-section-invisible branch February 7, 2023 11:20
@adrinjalali
Copy link
Copy Markdown
Member

@BenjaminBossan Does this mean users need to first add a section, then select it and then set the flag? It'd be nicer maybe to allow that from the start maybe? Users could even be adding a Section object?

@BenjaminBossan
Copy link
Copy Markdown
Collaborator Author

BenjaminBossan commented Feb 27, 2023

@adrinjalali Yes, basically there is no user API for making a section invisible yet. We can think about what would be the best way to do it. For now, I'm not sure if it needs to be exposed to users, let's perhaps wait and see if there is a need?

At the moment, this flag is used only for the model card space, because it allows to undo the deletion of a section. If we deleted a section by deleting it from _data and then try to undo that by adding it back to _data, the order is changed, which is what I wanted to prevent.

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

Successfully merging this pull request may close these issues.

3 participants