-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add csv import option #34
Conversation
Codecov Report
@@ Coverage Diff @@
## main #34 +/- ##
==========================================
- Coverage 31.70% 30.33% -1.37%
==========================================
Files 12 12
Lines 634 679 +45
==========================================
+ Hits 201 206 +5
- Misses 433 473 +40
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Probably good to move this conversation here |
Flattening callback functions if required by tests. Decided to abandon in favour of a selenium click on the upload button to test PR #34. This layout seems to work (see home callback for an example). @dash.callback instead of @app.callback. https://stackoverflow.com/a/72478423/1444054
4e43cad
to
97f5931
Compare
This reverts commit 39968c2.
…from sample_project_1)
cfac69f
to
b2e22ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I managed to get an error uploading our test xlsx. I suspect it's a fringe problem that Sanna won't encounter and I'd be inclined to merge this and tag a new version for her to pick up the functionality.
So let me approve, subject to you taking a quick look...
- @sfmig has taken a quick look.
In case you have the integration test working and want to add it here, just shout and I can re-review.
…efusedxml as recommended for security by openpyxl.
thanks for having a look @samcunliffe! I think the issue with the sample file you sent is now solved. It seems I needed to add openpyxl to the environment. Would you mind checking if it works now for you? There was an issue with the latest release of the Github actions (released 1h ago), so I reverted to the previous one to pass the checks. Let me know if there is a problem with this. If all is ok (🍉 ?), I will go ahead and merge, and then tag a new version for Sanna |
According to semver this should be v0.2.0 (because new feature != bugfix). |
Yes.
Probably doesn't hurt to make an |
Add a button to generate yaml files from a spreadsheet (csv or excel).
The yaml files are created in the video directory specified in the project config.
If there are metadata fields that are defined in the metadata_fields.yaml but are not columns in the spreadsheet, these are added to the video metadata files with empty values.
Right now no warning is issued if there are columns in the spreadsheet that are not defined in metadata_fields.yaml, but maybe this is something we should do (added as #77 )