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

Add csv import option #34

Merged
merged 13 commits into from
Apr 17, 2023
Merged

Add csv import option #34

merged 13 commits into from
Apr 17, 2023

Conversation

sfmig
Copy link
Collaborator

@sfmig sfmig commented Feb 15, 2023

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 )

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2023

Codecov Report

Merging #34 (50dd3f9) into main (61cb8a5) will decrease coverage by 1.37%.
The diff coverage is 14.58%.

@@            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     
Impacted Files Coverage Δ
wazp/callbacks/home.py 43.33% <ø> (ø)
wazp/pages/01_metadata.py 100.00% <ø> (ø)
wazp/pages/home.py 100.00% <ø> (ø)
wazp/callbacks/metadata.py 19.44% <14.58%> (-5.96%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sfmig
Copy link
Collaborator Author

sfmig commented Feb 15, 2023

Probably good to move this conversation here

@sfmig sfmig mentioned this pull request Mar 1, 2023
4 tasks
samcunliffe added a commit that referenced this pull request Apr 13, 2023
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
@sfmig sfmig marked this pull request as ready for review April 14, 2023 09:51
@sfmig sfmig force-pushed the add-csv-import-option branch from 4e43cad to 97f5931 Compare April 14, 2023 10:04
@sfmig sfmig force-pushed the add-csv-import-option branch from cfac69f to b2e22ce Compare April 14, 2023 10:59
@sfmig sfmig requested a review from samcunliffe April 14, 2023 11:06
Copy link
Member

@samcunliffe samcunliffe left a 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.

wazp/callbacks/metadata.py Show resolved Hide resolved
wazp/callbacks/metadata.py Show resolved Hide resolved
@sfmig
Copy link
Collaborator Author

sfmig commented Apr 17, 2023

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

@samcunliffe samcunliffe merged commit 0ddc85f into main Apr 17, 2023
@samcunliffe
Copy link
Member

samcunliffe commented Apr 17, 2023

According to semver this should be v0.2.0 (because new feature != bugfix).
I leave you to push the tag if that's ok. You should more pypi admin rights than me.

@samcunliffe
Copy link
Member

Would you mind checking if it works now for you?

Yes.

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.

Probably doesn't hurt to make an upstream label for issues related to the cookie-cutter template and/or GH actions. What do you think?

@samcunliffe samcunliffe deleted the add-csv-import-option branch April 17, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants