[FIX] change remaining latin expressions (etc and i.e.) #628

Remi-Gau · 2020-10-01T03:49:33Z

TO DO

fix remaining latin expression (etc and i.e)
lint tables
lint paragraphs
- enforce line length
- start sentences in a new line if the modified paragraph

for reviewers

This one is more complicated than for e.g.. I am not sure that some of the changes I made "read well": suggestions welcome.

I will get on to the linting once we agree on the content, if that is fine with you

This will most likely create merge conflicts after #626 is merged (because it seems Latin expressions tend to live in pack and cluster), so let me know whether we should:

merge [FIX] replace "e.g.," by "for example" #626 first and then solve conflicts (my personal preference)
"extend" [FIX] replace "e.g.," by "for example" #626 with the current changes and have one single PR for all the "Latin busting" changes.

Not sure that the specs have a position on the sempiternal "Oxford comma" debate... But this PR might raise the question. 😝

sappelhoff

Thanks @Remi-Gau!

This one is more complicated than for e.g.. I am not sure that some of the changes I made "read well": suggestions welcome.

I would more often simply drop "etc." completely instead of replacing it with "and so on"

merge #626 first and then solve conflicts (my personal preference)

fine with me 👍

Not sure that the specs have a position on the sempiternal "Oxford comma" debate... But this PR might raise the question. stuck_out_tongue_closed_eyes

I use that kind of comma and feel like it helps the reading flow, but I never dove into the debate 🤷‍♂️ ... yet I would be a strong +1 on using it (also in this PR)

Let's wait for another reviewer (at least one) to confirm / discomfirm my opinions ... then make the changes (or not) ... then review in detail :-)

Remi-Gau · 2020-10-01T08:56:47Z

On related issue: would it make sense to "install" the github action used by the Turing way to check our content for Latin expressions to ensure we don't have to redo this in the future?

sappelhoff · 2020-10-01T09:02:01Z

On related issue: would it make sense to "install" the github action used by the Turing way to check our content for Latin expressions to ensure we don't have to redo this in the future?

probably worth looking into, good idea! 👍 we'd have to clarify what else they are linting / checking, and make sure that all of these steps are (or can be) configured to our liking.

If you have time and interest to do that, I'd appreciate a short report and potentially a new PR :-)

Remi-Gau · 2020-10-02T13:23:59Z

Will mark this as ready for review to see how many conflicts we have.

effigies · 2020-10-05T20:05:18Z

Will mark this as ready for review to see how many conflicts we have.

effigies · 2020-10-05T21:08:07Z

For the sake of not breaking all PRs, I commented out i.e. and etc in 11caf05. When you're ready to review, make sure you're based off of master and uncomment those. I will go through and comment on wording now.

effigies

Some opinions.

CONTRIBUTING.md

src/02-common-principles.md

src/04-modality-specific-files/05-task-events.md

src/99-appendices/01-contributors.md

src/99-appendices/05-units.md

src/99-appendices/06-meg-file-formats.md

src/99-appendices/09-entities.md

src/02-common-principles.md

src/03-modality-agnostic-files.md

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md

src/04-modality-specific-files/02-magnetoencephalography.md

src/04-modality-specific-files/03-electroencephalography.md

src/04-modality-specific-files/04-intracranial-electroencephalography.md

src/99-appendices/06-meg-file-formats.md

src/99-appendices/09-entities.md

Remi-Gau · 2020-10-06T05:30:07Z

src/04-modality-specific-files/04-intracranial-electroencephalography.md

@@ -253,7 +253,7 @@ SHOULD be present:
 | reference          | OPTIONAL              | Specification of the reference (e.g., 'mastoid', 'ElectrodeName01', 'intracranial', 'CAR', 'other', 'n/a'). If the channel is not an electrode channel (e.g., a microphone channel) use `n/a`.                                                                             |
 | group              | OPTIONAL              | Which group of channels (grid/strip/seeg/depth) this channel belongs to. This is relevant because one group has one cable-bundle and noise can be shared. This can be a name or number. Note that any groups specified in `_electrodes.tsv` must match those present here. |
 | sampling_frequency | OPTIONAL              | Sampling rate of the channel in Hz.                                                                                                                                                                                                                                        |
-| description        | OPTIONAL              | Brief free-text description of the channel, or other information of interest (e.g., position (e.g., "left lateral temporal surface", etc.).                                                                                                                                |
+| description        | OPTIONAL              | Brief free-text description of the channel, or other information of interest (e.g., position (e.g., "left lateral temporal surface")).                                                                                                                                |


is it me and my sleep deprivation, but aren't these nested parenthesis hard to parse for the average human being?

I agree with you, and I just had 8 hours of sound sleep :-)

should be easy to avoid nested brackets in this case

I think that I had a similar thought at one point and tried switching to square brackets nested in the parentheses, but that doesn't play well with markdown (or maybe one of the other formatters that the specification uses).

Remi-Gau · 2020-10-06T06:37:16Z

Well... That was "fun"... Let's NEVER do that again.

When we are OK on content, I will finalize the linting of the tables.

effigies

Here are my suggestions.

src/02-common-principles.md

src/04-modality-specific-files/01-magnetic-resonance-imaging-data.md

src/99-appendices/01-contributors.md

src/schema/entities.yaml

src/04-modality-specific-files/02-magnetoencephalography.md

src/04-modality-specific-files/03-electroencephalography.md

src/04-modality-specific-files/04-intracranial-electroencephalography.md

effigies · 2020-10-06T15:46:24Z

Closing/reopening to re-run the RTD build.

effigies · 2020-10-06T16:32:52Z

@Remi-Gau Do you mind if I just quickly push a few formatting changes, rather than another round of suggestions?

Remi-Gau · 2020-10-06T16:33:57Z

@Remi-Gau Do you mind if I just quickly push a few formatting changes, rather than another round of suggestions?

@effigies Be my guest !!!!

effigies · 2020-10-06T16:47:03Z

The first commit was just sed -i -e 's/[\t ]*$//' src/**/*.md, so it went a bit out of scope for the PR. Can do it on master, if we prefer.

effigies

I'm happy with this PR.

sappelhoff

LGTM! Thanks @Remi-Gau and @effigies

edit: seems like I introduced a conflict by merging the HED PR first. I'll fix that.

Co-authored-by: Chris Markiewicz <[email protected]>

- ignore pregh-changes.md - checks for all lating expressions

Co-authored-by: Chris Markiewicz <[email protected]>

Remi-Gau added this to the 1.4.1 milestone Oct 1, 2020

sappelhoff reviewed Oct 1, 2020

View reviewed changes

Remi-Gau closed this Oct 1, 2020

Remi-Gau reopened this Oct 1, 2020

Remi-Gau mentioned this pull request Oct 1, 2020

Setup a no-bad-latin github action #635

Closed

Remi-Gau marked this pull request as ready for review October 2, 2020 13:23

Remi-Gau requested review from chrisgorgo and monkeyman192 as code owners October 2, 2020 13:23

Remi-Gau mentioned this pull request Oct 2, 2020

[FIX] URI "definition" and recommendation #629

Merged

2 tasks

effigies reviewed Oct 5, 2020

View reviewed changes