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

ENH: Add possibility to turn section invisible #288

Conversation

BenjaminBossan
Copy link
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
Collaborator Author

@skops-dev/maintainers ready for review

Copy link
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
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
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