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

Replace dashes with underscores #307

Open
ndporter opened this issue Aug 8, 2023 · 9 comments
Open

Replace dashes with underscores #307

ndporter opened this issue Aug 8, 2023 · 9 comments
Labels
good first issue Good issue for first-time contributors help wanted Looking for Contributors

Comments

@ndporter
Copy link

ndporter commented Aug 8, 2023

Following best practices (see TidyData lesson column headers being created should use _ instead of - whenever possible. This applies at least to Looking up Data (e.g., Journal-details) but other episodes should be checked as well for consistency.

@ostephens ostephens added help wanted Looking for Contributors good first issue Good issue for first-time contributors labels Aug 8, 2023
@ostephens
Copy link
Contributor

ostephens commented Aug 8, 2023

Great suggestion @ndporter - thank you 🙏

If anyone is willing / able to check through the lesson and identify any other examples, or even better make the edits for the maintainers to then approve that would be amazing.

This tutorial shows how you can show us the exact line and make the edit for maintainers to approve

@ndporter
Copy link
Author

ndporter commented Aug 8, 2023

I won't have a chance to do this in the near future, or I would have put in a PR myself. I will also note that images may need to be changed to be consistent with the text (including the example I identified).

@jas58
Copy link
Contributor

jas58 commented Aug 12, 2023

I'll add a note to also make the hyphen in User-Agent an underscore.

And a question: Might this be an instructor note in addition to the text edits?

Also, what does the alt-text need to be? it is currently:
Add column by fetching URLs screen capture

Does the alt-text need to describe what the image looks like (but doesn't need to repeat preceding or following instructions):
"The preview panel should read Row 1's value is 1099-4300 and the third column shows the api.crossref address completed."

@bencomp
Copy link
Contributor

bencomp commented Aug 14, 2023

I'll add a note to also make the hyphen in User-Agent an underscore.

If you meant the HTTP header: that should be a hyphen.

@ndporter
Copy link
Author

Good catch @bencomp it's important to only change the column headers we create, not the data we're trying to pass through the API, whose structure is determined by the providers.

mahdi-robbani added a commit to mahdi-robbani/lc-open-refine that referenced this issue Aug 24, 2023
mahdi-robbani added a commit to mahdi-robbani/lc-open-refine that referenced this issue Aug 24, 2023
ostephens added a commit that referenced this issue Aug 28, 2023
#307 Replace dashes with underscores (13-looking-up-data.md)
github-actions bot pushed a commit that referenced this issue Aug 28, 2023
Auto-generated via {sandpaper}
Source  : 1aef633
Branch  : main
Author  : Owen Stephens <owen@ostephens.com>
Time    : 2023-08-28 16:13:12 +0000
Message : Merge pull request #310 from mahdi-robbani/main

#307 Replace dashes with underscores (13-looking-up-data.md)
github-actions bot pushed a commit that referenced this issue Aug 28, 2023
Auto-generated via {sandpaper}
Source  : a2e24f0
Branch  : md-outputs
Author  : GitHub Actions <actions@github.com>
Time    : 2023-08-28 16:14:37 +0000
Message : markdown source builds

Auto-generated via {sandpaper}
Source  : 1aef633
Branch  : main
Author  : Owen Stephens <owen@ostephens.com>
Time    : 2023-08-28 16:13:12 +0000
Message : Merge pull request #310 from mahdi-robbani/main

#307 Replace dashes with underscores (13-looking-up-data.md)
@ostephens
Copy link
Contributor

ostephens commented Aug 28, 2023

Column names in "Looking up data" episode fixed by #310. Waiting on confirmation from @mahdi-robbani whether a review of all episodes was included in that work

github-actions bot pushed a commit that referenced this issue Aug 29, 2023
Auto-generated via {sandpaper}
Source  : a2e24f0
Branch  : md-outputs
Author  : GitHub Actions <actions@github.com>
Time    : 2023-08-28 16:14:37 +0000
Message : markdown source builds

Auto-generated via {sandpaper}
Source  : 1aef633
Branch  : main
Author  : Owen Stephens <owen@ostephens.com>
Time    : 2023-08-28 16:13:12 +0000
Message : Merge pull request #310 from mahdi-robbani/main

#307 Replace dashes with underscores (13-looking-up-data.md)
@ostephens
Copy link
Contributor

Confirmed that #310 only addresses the Looking up data episode, so there is still a task here to check if there are any column names that are not appropriately formatted in any other episodes

@ostephens
Copy link
Contributor

Have added #314 for the user-agent/http header issue

@ostephens
Copy link
Contributor

Remaining tasks for this issue:

  • Review all episodes to find any other column names suggested/used and ensure they are consistent with good practice as per the tidy data lesson (in all aspects, but including specifically using underscores not hyphens). Make sure to also take note of any screen illustrations that need updating as a result
  • Update screen illustration in https://librarycarpentry.org/lc-open-refine/13-looking-up-data.html to have the underscore version of the column name rather than hyphenated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for first-time contributors help wanted Looking for Contributors
Projects
None yet
Development

No branches or pull requests

4 participants