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

Proposal: additive configure #66

Open
cccntu opened this issue Jan 11, 2021 · 2 comments
Open

Proposal: additive configure #66

cccntu opened this issue Jan 11, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@cccntu
Copy link

cccntu commented Jan 11, 2021

  • nbautoexport version: 0.2.1
  • Python version: 3.9.1
  • Operating System: Ubuntu 18.04.1

Description

In the clean doc: https://nbautoexport.drivendata.org/cleaning/
I tried to add a folder to exclude, and it overwrote my old non-default export_formats config.
I think it would be more natural to not have everything overwritten.

What I Did

❯ cat notebooks/.nbautoexport
{
  "export_formats": [
    "script",
    "html"
  ],
  "organize_by": "extension",
}
❯ nbautoexport configure notebooks/ \
    --clean-exclude 'report/*'
Detected existing autoexport configuration at notebooks/.nbautoexport. If you wish to overwrite, use the --overwrite flag.
❯ nbautoexport configure notebooks/ --overwrite \
    --clean-exclude 'report/*'
❯ cat notebooks/.nbautoexport
{
  "export_formats": [
    "script"
  ],
  "organize_by": "extension",
  "clean": {
    "exclude": [
      "report/*"
    ]
  }
}
@jayqi jayqi added the enhancement New feature or request label Feb 20, 2021
@jayqi
Copy link
Member

jayqi commented Feb 20, 2021

There are two ways I see this going:


1. Always replace a configured option.

{
  "export_formats": [
    "script"
  ],
  "organize_by": "extension",
}

nbautoexport configure notebooks/ -f html

would give you

{
  "export_formats": [
    "html"
  ],
  "organize_by": "extension",
}

2. Lists are additive

{
  "export_formats": [
    "script"
  ],
  "organize_by": "extension",
}

nbautoexport configure notebooks/ -f html

would give you

{
  "export_formats": [
    "script",
    "html"
  ],
  "organize_by": "extension",
}

Not sure which kind of behavior is more intuitive and easy-to-use.

As for the old behavior of replacing a file entirely, we could always tie that to the --overwrite flag or a new flag like --replace or something.

@cccntu
Copy link
Author

cccntu commented Feb 22, 2021

Update: I no longer think there is a need for this functionality. I just directly edit the config file like other dotfiles.

I think we just need to prompt user to optionally edit the config file themselves when existing file is detected. Maybe add documentation for the syntax.

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

No branches or pull requests

2 participants