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

feat: add bed file datafetcher #877

Merged
merged 45 commits into from
May 6, 2023
Merged

Conversation

etowahadams
Copy link
Contributor

@etowahadams etowahadams commented Apr 19, 2023

Fix #583
Toward #870

Change List

  • Add support for BED files
  • Refactor CSV data fetcher

Test out using this gist: http://localhost:3000/?gist=etowahadams/cd49a7f308e4859e095d209056a564d3

Documentation PR

image

Checklist

  • Ensure the PR works with all demos on the online editor
  • Unit tests added or updated
  • Examples added or updated
  • Documentation updated (e.g., added API functions)
  • Screenshots for visual changes (e.g., new encoding support or UI change on Editor)

tsconfig.json Outdated Show resolved Hide resolved
package.json Outdated
@@ -155,7 +157,7 @@
},
"husky": {
"hooks": {
"pre-commit": "run-p changelog schema schema-higlass schema-theme schema-template format && git add .",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes I only want to commit things that are staged. When the pre-commit hook would run, it would add all my unstaged files

Copy link
Member

Choose a reason for hiding this comment

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

I see. Would the generated schema files and the changelogs still be included by the pre-commit hooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you're right. I'll add it back.

Copy link
Member

Choose a reason for hiding this comment

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

@manzt Do you aware of any better way to deal with this?

Copy link
Member

@manzt manzt May 5, 2023

Choose a reason for hiding this comment

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

I think this is something we could probably check in CI? For example, we could have a GitHub action that runs a package.json script and assert that there are no git changes. If there are, fail and require the developer to run locally in order to pass CI. This way we allow much more flexibility when creating PRs, but ensure the changelog and schemas must be in sync in order to be merged into master.

I personally really dislike pre-commit hooks because I commit often.

Copy link
Member

Choose a reason for hiding this comment

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

I like the idea! I also dislike waiting for the pre-commit hooks to run scripts.

Copy link
Member

@sehilyi sehilyi left a comment

Choose a reason for hiding this comment

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

This is awesome! I left several minor comments.

Also, do you want to add a simple example to our example gallery? That would be really great to include.

package.json Outdated
@@ -155,7 +157,7 @@
},
"husky": {
"hooks": {
"pre-commit": "run-p changelog schema schema-higlass schema-theme schema-template format && git add .",
Copy link
Member

Choose a reason for hiding this comment

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

I see. Would the generated schema files and the changelogs still be included by the pre-commit hooks?

src/core/init.ts Outdated Show resolved Hide resolved
src/data-fetchers/bed/bed-worker.ts Outdated Show resolved Hide resolved
src/data-fetchers/bed/bed-worker.ts Outdated Show resolved Hide resolved
src/data-fetchers/bed/bed-parser.ts Outdated Show resolved Hide resolved
src/data-fetchers/bed/bed-parser.test.ts Show resolved Hide resolved
src/data-fetchers/bed/bed-parser.ts Outdated Show resolved Hide resolved
src/data-fetchers/bed/shared-types.ts Outdated Show resolved Hide resolved
@etowahadams
Copy link
Contributor Author

etowahadams commented May 6, 2023

Finished making edits, just need to add the example.

@manzt manzt mentioned this pull request May 6, 2023
5 tasks
@etowahadams
Copy link
Contributor Author

Added example, ready to merge

@etowahadams etowahadams merged commit a087340 into master May 6, 2023
@etowahadams etowahadams deleted the etowahadams/make-bed-primitive branch May 6, 2023 19:03
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.

feat: Make BED v1 a primitive data format
3 participants