Simple Geometry Contribution

Dear CF,
This addition has been discussed on the email list heavily. It has not been submitted through trac as it seems that this ticket represents consensus to stop using trac in favor of github. I think it may be useful to consider a contribution not vetted in trac to pilot a workflow in github. If it is preferably to the community, I will circle back and use trac as in other contributions. Just trying to nudge the group and give a bit of food for thought.
Note that I have additional commit history that demonstrates peer review of the actual text on this branch. If the group would rather have a rich commit history, we can close this and open a PR from that branch.
For completeness of the history in case people come to this PR without having been part of the conversation on the email list, this work has been developed and vetted in a github repository here. The issue history in that repository contains nearly all the communication between the team that developed the proposal. The wiki in that repository also contains a more verbose description of the new section 7.5 being contributed here. That repository also contains a reference python implementation of the format. This repository contains an R implementation.
Best,
Dave
See https://cf-trac.llnl.gov/trac/ticket/160

[ ] Added link from trac ticket to this PR?

Applied via https://github.com/cf-convention/cf-conventions/pull/XXX

[x] Updated "Revision History"? (Use the date you applied the changes.)

Dear Dave
Thanks for doing this. I think it's in pretty good shape but I have quite a few detailed comments and suggestions to make, nearly all on the proposed text rather than the convention itself. I'm not sure how to do that in GitHub, so it's a useful exercise to see how this would work. If your proposal was in trac, I would reply to your posting on the ticket, edit the wiki-markup text to show my changes and suggestions for the parts affected, and repost it to the trac ticket. If it was on a wiki, I would make a copy of it on the wiki page, and edit it similarly.
The way GitHub is set up, I suppose the natural way to do it is to make a new branch and edit that, but (a) I don't know how to do that, (b) it's not obvious to me that the changes I suggested would be clear to you. In fact I find the proposal in this form not as easy to follow as it would be in trac. I can view it as deltas of the files, but these have little context and are hard to read as text because the markup isn't translated, or I can read the properly rendered modified files, but these don't show what's been changed, and of course they show much more that isn't affected, especially as it's in several different complete files.
So I'm inclined to think that it would be easier to use GitHub issues in the same way as we use trac. That is, you would post your entire proposed text to the "issue". Then I presume I could copy your posting and edit it, as in trac. Unfortunately, the markup isn't the same, is it - the issues use markdown, I believe, whereas the convention text uses AsciiDoc. This is a technical obstacle. Is there an automatic translator? If not, once the text is agreed, it would have to be manually transposed into the conventions document, as we have been doing from agreed trac tickets.
You and others have much more experience with this, so perhaps you could suggest how to proceed, both in general with using GitHub issues (but that might be better on ticket 160) and specifically to give you detailed suggestions for your text.
Best wishes and thanks
Jonathan

Dear Jonathan,
I agree that this is a useful test case for how we might do this in github. I somewhat intentionally submitted this in a way that would provoke such discussion.
I've been pondering this a bit and I think that the path of least resistance may be to submit an issue rather than a pull request highlighting additions or changes. I'll close this pull request and open an Issue instead. That way we can use markdown to document text review.
My suggestion would be to use strikethrough text (~~text wrapped in double tilde~~)  to show that text is to be changed, and bold text (**text wrapped in double asterisk**)  to indicate new additions.
Once the text is finalized in an issue, someone would submit a pull request like this one that includes the finalized content. That way the PR review will be about the details of the asciidoc and not the details of the proposed change to the CF spec information content.
Best,

Dave

hmm -- I"m going to have to disagtee here -- if there is more than a small amount of text, then a
PR is really a better way to deal with it.
@JonathanGregory posted in a TRAC ticket, which then ended up on the mailing list, where I read it, but I can't comment on TRAC tickets, so I'll address the issues here:

I have
quite a few detailed comments and suggestions to make, nearly all on the
proposed text rather than the convention itself.

With a PR, it is easy to make comments on individual lines of text -- this is a really nice way to intersperse comments and particularly when it is small word-smithing issue or whatever. And you can also post larger comments that are not tied to individual parts of the text. Not sure what the confusion on this is -- this particular use case is really well supported by gitHub PRs and not nearly as well supported by dumping a large block of text in an issue.
Go to the "Files Changed" tab on the PR, and you can click on any line of text and add a comment there.

The way !GitHub is set up, I suppose the natural way to do it is to make
a new branch and edit that, but (a) I don't know how to do that, (b) it's
not obvious to me that the changes I suggested would be clear to you.

A workflow like this would be the way to go if you wanted to suggest large changes to the text. IN this case, you are moving to using git (rather than gitHub per-se) features -- then you would want to create a new branch or fork, and make your changes, and then submit them as a PR. This required a fair bit of git-fu -- but yes, the changes would be very clear. Perhaps we should write up a tutorial on how to do that.
BTW -- there are (kinda) two workflows for this kind of thing. In general, gitHub supports two kinds of contributions:
(A) Contributions from the "core team" -- in that case, these folks have permissions to push to the main gitHub repository. Then they can either: directly make a change, or (better option for non-tirivial changes) create a branch, make the changes there, and then do a PR to merge the branch back into master -- it can then be reviewed and discussed by the team before merging.
(B) Contributions from non-members of the core team. In this case, the workflow is to fork the repository, make the changes in that fork, and do a PR to the main repo to merge. The changes are then discussed and reviewed, and can be merged in when ready.
These seem very similar, but (B) can be a quite a bit more awkward if others want to do more than comment, and actually go in and make changes to the file in question.
If you are using workflow (A) then any core member can pull the branch, and look at it, and make changes an push them up -- so a smaller group can collaborate on a set of changes easily.
If you are using workflow (B) (as is the case here), the then "new" stuff is in a fork of the repo in someone else's gitHub account (@dblodgett-usgs in this case). So no one else has permission to edit that -- we can comment on the PR, but only the proposer can actually change anything. So if you do want to edit the doc itself and suggest those changes, you need to fork the forks' repo, and do it there, and then submit a PR to that repo -- this gets pretty complicated, though It's not actually that hard to do.

I can view it as deltas of the files, but these have little
context and are hard to read as text because the markup isn't translated,

I guess ASCIIDOC is not as fabulously readable as we'd like :-(
As for the context, that is a gitHub limitation -- you can either see only the changes, without much context (but able to comment on it line by line) -- or see the whole file, but then teh cnages aren't highlighted and you can't comment line by line. However, I find it only  slightly painful to switch back and forth -- look at the changes, and if I need context, switch to the whole file to see what's going on and then back again to comment.

or I can read the properly rendered modified files, but these don't show
what's been changed, and of course they show much more that isn't
affected, and it's several different complete files.

I can't really see how this is made better by jamming all the text into the issue. -- now to see the changes, the proposer is going to have to mark it up by hand somehow....

Unfortunately, the markup isn't the same, is it - the issues use
markdown, I believe, whereas the convention text uses !AsciiDoc. This is a
technical obstacle. Is there an automatic translator?

It looks like gitHub does know how to render asciidoc:
https://github.com/dblodgett-usgs/cf-conventions/blob/94875c55a97596c9055174244714baaa1d9ffee4/appe.adoc
BTW, you can open different "views" of the same PR in multiple tabs in your browser. This can be very helpful if you want to look at the changes and make comments, and also switch quickly and easily to the full page to see context, for example.
In short -- let's give gitHub a chance! -- This kind of PR review is one of the features we WANTED from gitHub.

@dblodgett-usgs wrote:

My suggestion would be to use strikethrough text (text wrapped in double tilde) to show that text is to be changed, and bold text (text wrapped in double asterisk) to indicate new additions.

That's exactly the kind of hand-editing that I think is a big waste of time -- let the computer tell you what's changed! This is a lot of work for the proposer

Once the text is finalized in an issue, someone would submit a pull request like this one that includes the finalized content. That way the PR review will be about the details of the asciidoc and not the details of the proposed change to the CF spec information content.

I guess I'm confused about where this proposal is at -- if it's still about the CF spec issues, then yes, a Issue may be a better way to discuss -- but then there shouldn't be any asciidoc required at all -- or ADDING markup for changes, etc.
When Jonathan wrote:

quite a few detailed comments and suggestions to make, nearly all on the
proposed text rather than the convention itself"

That sounds to me like we ARE talking about details of text and format, and a PR is the best way to manage that stage of the process.

This is all really good analysis. Thanks! I may have (probably did) swing too far with hand-markup of text changes.
Two things:
I totally forgot about line-by-line comments in the PR diff. I think that may be the solution @JonathanGregory is looking for.
This proposal is at the point of wordsmithing and minor details. It does not HAVE to be at the point of a formatted contribution to the repository that get's pushed out to cfconventions.org. I'm not sure that distinction is worth getting hung up on, but maybe it is?
So, I'll reopen this and we can give commenting on the diff a chance.

Hi David and Chris,
I'm a bit lost - how can we make/view diffs of proposed text, such as that
given at https://github.com/cf-convention/cf-conventions/issues/112
Thanks!
David
On 3 May 2017 at 18:02, David Blodgett<EMAIL_ADDRESS>wrote:

This is all really good analysis. Thanks! I may have (probably did) swing
too far with hand-markup of text changes.
Two things:
I totally forgot about line-by-line comments in the PR diff. I think that
may be the solution @JonathanGregory https://github.com/JonathanGregory
is looking for.
This proposal is at the point of wordsmithing and minor details. It does
not HAVE to be at the point of a formatted contribution to the repository
that get's pushed out to cfconventions.org. I'm not sure that distinction
is worth getting hung up on, but maybe it is?
So, I'll reopen this and we can give commenting on the diff a chance.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/cf-convention/cf-conventions/pull/109#issuecomment-298972768,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHwAcJro-80e-nLq3Ad7SAlhfmTeSnphks5r2LMqgaJpZM4NFFoo
.

--
David Hassell
National Centre for Atmospheric Science
Department of Meteorology, University of Reading,
Earley Gate, PO Box 243, Reading RG6 6BB
Tel: +44<PHONE_NUMBER>
http://www.met.reading.ac.uk/

Line by line comments are possible in the diff view of a pull request. Click on the line number on this page: https://github.com/cf-convention/cf-conventions/pull/109/files?diff=unified .
You should see something like this:

On Wed, May 3, 2017 at 10:11 AM, David Hassell<EMAIL_ADDRESS>wrote:

Hi David and Chris,
I'm a bit lost - how can we make/view diffs of proposed text, such as that
given at https://github.com/cf-convention/cf-conventions/issues/112

That is an "Issue", rather than a PR (Pull Request) -- and there is not way
to make/view/comment on diffs in an issue -- and Issue is really just a
conversation.
Which is why, for this kind of thing, a PR is a good option -- in that
case, someone is proposing actuall cahnges to the doc, and you can see teh
changes, comment on them line by line, etc:
if you go here:
https://github.com/cf-convention/cf-conventions/pull/109
then click on the "Files Changed" tab -- you will see the changes, and can
comment on them.
There have got to be a lot of tutorials on the web about the details...
-CHB

Thanks!
David
On 3 May 2017 at 18:02, David Blodgett<EMAIL_ADDRESS>wrote:

This is all really good analysis. Thanks! I may have (probably did) swing
too far with hand-markup of text changes.
Two things:
I totally forgot about line-by-line comments in the PR diff. I think that
may be the solution @JonathanGregory <https://github.com/JonathanGregory
is looking for.
This proposal is at the point of wordsmithing and minor details. It does
not HAVE to be at the point of a formatted contribution to the repository
that get's pushed out to cfconventions.org. I'm not sure that
distinction
is worth getting hung up on, but maybe it is?
So, I'll reopen this and we can give commenting on the diff a chance.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/cf-convention/cf-conventions/pull/109#issuecomment-
298972768>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHwAcJro-80e-
nLq3Ad7SAlhfmTeSnphks5r2LMqgaJpZM4NFFoo>
.

--
David Hassell
National Centre for Atmospheric Science
Department of Meteorology, University of Reading,
Earley Gate, PO Box 243, Reading RG6 6BB
Tel: +44<PHONE_NUMBER> <+44%20118%20378%205613>
http://www.met.reading.ac.uk/
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/cf-convention/cf-conventions/pull/109#issuecomment-298974998,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA38YFt4Xl95LcHnesAgbTegxa192Bdqks5r2LUmgaJpZM4NFFoo
.

--
Christopher Barker, Ph.D.
Oceanographer
Emergency Response Division
NOAA/NOS/OR&R<PHONE_NUMBER>   voice
7600 Sand Point Way NE<PHONE_NUMBER>   fax
Seattle, WA  98115<PHONE_NUMBER>   main reception
<EMAIL_ADDRESS>

Thanks for this discussion. In trac, I would have to do quite a lot of work as well to propose changes and make comments in a clear way, and I'd of course prefer it to be automated as far as possible. What I want to do in this case is about word-smithing, and just a few points about the detail of the proposal. Writing it all as comments is inefficient e.g. "I suggest changing A to B". It is much easier for me to change A to B in the text, and if it's not obvious why I would have suggested that, I would add a comment as well. So I want to make changes and comments, like one would in a Word document with tracked changes. Of course I am not suggesting that dreadful and hideous alternative, but is there a way to do this in GitHub? So far it sounds like that can't be done without forking a branch, but I fear that would be too much hard work for everyone who might want to contribute to debates about CF text as they do in trac tickets.

I think the easiest might be to use this view select the line you want to modify, and go to town. I've added a comment over there. Here's a link to the comment.

I had assumed that edits to CF are frequent enough and complicated enough, that it would make sense to make a branch for each proposed change, on which all of the discussions and changes will occur, which then get merged to the master version once the final version is agreed to.
(note: I am familiar with github, but am not an expert)

I think "complicated enough" is key -- if's a typo fix, then don't bother, but larger edits should get a branch.
However, you can only really do a branch if yu have commit rights to the main repo. So anyone that doesn't can to a PR instead, which is pretty much the same thing.
(and a PR can be merged into a branch as well -- it doesn't have to go straight to master -- if there is a big change that needs further review by the core contributors.

Hi @ChrisBarker-NOAA , I normally think of a PR as the mechanism to merge one branch with another (usually on to the master branch).  How does a PR work without a branch?
Is this discussion actually happening on a branch, because it is a little different than I am used to?

A PR can be for a merge of one branch to another within the same project -- but that can only be done by people that have write permissions to the central repo.
A PR from a fork, like this one, is essentially the same, but the different "branch" is in someone else's repo (in this case  dblodgett-usgs's account) as it is a different repo, it doesn't matter what it's called "master" in dblodgett-usges' repo is separate from "master" in this repo.
clear as mud?

@ChrisBarker-NOAA , so the real distinction is between a branch and a fork?

@ChrisBarker-NOAA https://github.com/ChrisBarker-NOAA , so the real
distinction is between a branch and a fork?

yup.

Thanks.    BTW, I am submitting this reply by responding via email, to show what it looks like.

Philip Cameron-Smith<EMAIL_ADDRESS>CliMA Group Leader, Lawrence Livermore Nat. Lab.
From: Chris Barker<EMAIL_ADDRESS>Sent: Friday, May 05, 2017 2:30 PM
To: cf-convention/cf-conventions<EMAIL_ADDRESS>Cc: Cameron-smith, Philip<EMAIL_ADDRESS>Comment<EMAIL_ADDRESS>Subject: Re: [cf-convention/cf-conventions] Simple Geometry Contribution (#109)

@ChrisBarker-NOAA https://github.com/ChrisBarker-NOAA , so the real
distinction is between a branch and a fork?

yup.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com/cf-convention/cf-conventions/pull/109#issuecomment-299580525, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIkyUEKhm2hNX0wCNFE8IFQ9hBt7laZcks5r25TkgaJpZM4NFFoo.

The group needs to decide if a branched workflow is going to be helpful. I prefer the fork / pull request onto master workflow because it grants full control of the fork to the person making changes, but I’d be open to other alternatives as discussed above.
Note that some email client reply headers work better with GitHub than others. My Mac mail client is usually pretty tidy, but it looks like whatever is used at LLNL isn’t parsed out nicely by GitHub.

On May 5, 2017, at 7:13 PM, cameronsmith1<EMAIL_ADDRESS>wrote:
Thanks. BTW, I am submitting this reply by responding via email, to show what it looks like.

Philip Cameron-Smith<EMAIL_ADDRESS>CliMA Group Leader, Lawrence Livermore Nat. Lab.
From: Chris Barker<EMAIL_ADDRESS>Sent: Friday, May 05, 2017 2:30 PM
To: cf-convention/cf-conventions<EMAIL_ADDRESS>Cc: Cameron-smith, Philip<EMAIL_ADDRESS>Comment<EMAIL_ADDRESS>Subject: Re: [cf-convention/cf-conventions] Simple Geometry Contribution (#109)

@ChrisBarker-NOAA https://github.com/ChrisBarker-NOAA , so the real
distinction is between a branch and a fork?

yup.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHubhttps://github.com/cf-convention/cf-conventions/pull/109#issuecomment-299580525, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AIkyUEKhm2hNX0wCNFE8IFQ9hBt7laZcks5r25TkgaJpZM4NFFoo.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/cf-convention/cf-conventions/pull/109#issuecomment-299601914, or mute the thread https://github.com/notifications/unsubscribe-auth/ABbHQ8GtO2IKDSnABaZh2FtBML0BrZ8Kks5r27sSgaJpZM4NFFoo.

The group needs to decide if a branched workflow is going to be helpful. I prefer the fork / pull request onto master workflow because it grants full control of the fork to the person making changes,

Wry good point -- I think which is best is situation dependent -- a
fork-based workflow provides better control to the proposer. A
branched-based workflow provides easier collaboration on a change.
Got is very flexible though, so if there is a small subgroup that
wants to collaborate on a change, then multiple people can be granted
access to the forked version.
I tend to prefer the branch-beaded workflow, but it does require
giving permission to more people to push to the many repo, which may
be too much admin overhead.
-CHB

When I was looking at the current Pull Request in Github, there were various options that it wouldn't let me access because I didn't have permission (presumably because it is a fork).   What would it take to give people permission to edit the current Fork/PR, so people can see the editing features that will be available?
(FYI, I am responding by email again, but this time I am stripping off everything else in my email.)

The form belongs to @dblodgett-usgs -- so he would have to give yu permission to edit his repo.
Or you can fork his fork -- make changes and do a PR against his fork -- this gets complicated fast!
-CHB

As I understand it, then, there are 4 options for handling changes:

People will provide feedback on a proposed change only by making comments.
Give a lot of people write-access to the main CF git repository, and use branches.
Someone proposing a change will fork the git repository, and give write-access to people (probably a select few).
Someone proposing a change will fork the git repository, and others wanting to alter the proposed changes will fork the fork, then issue a new pull request.

Personally, I think (1) will be limiting, and (4) would get very messy and confusing for a large and controversial change.

agreed, though:

is not that limiting, and in this case @dblodgett-usgs suggested that he wanted to make any changes himself.
required write access to the core folks working on a change -- I don't know how practical that is.
is a fine option -- a major change may be getting proposed by a small group of folks -- make sense for them (and only them) to share a repo to do it.

I agree - (4) is pretty unweildy.
For small changes, (1) is fine.
For larger changes, (3) should work.

I'll make the observation that, for most large submissions, only one person is going to be the editor of the text. So, like in this case, putting in a PR from a fork (owned by the submitting editor) works well.
In this case, I forked the repository, added my draft contribution, had two collaborators review it (which you can see in these two closed PRs), then submitted the PR here.
My intention is to take all the comments I receive and reconcile them into the open PR with a new commit. When I do that, you will see that comments on old (now stale) content will be hidden and the review process can continue fresh.
So the way I've done this, which has been completely natural and easy, has been a mix of 1 (for community review) and 4 (for peer review prior to the community). Note that for this proposal we had significant community review on the email list and in another github repository / google docs as well prior to getting to this point. We are NOT starting from scratch with a github pull request.
On a somewhat unrelated not, I don't think pushing to the shared group repository should ever be allowed. Curious how those who have been maintaining this repo feel about it. IMHO, all changes should come in through pull requests. This keeps the requirement for review on all community members. Pulling commits from/to each other's forks is not all that hard once you have multiple remote repositories in your local copy.
Finally, I'm looking forward to @JonathanGregory's line by line critique of this work. After all this conversation, it seems that the pull request review feature is really what you are looking for. When you go to make a comment on a line there is an option to "start a review". It would be great to have one comprehensive review from you. Maybe we would have a comment that at-least two community members have completed a peer review of the material prior to merging?

Note that there is now an "outdated" comment thread from @rsignell-usgs above. The commit just above this comment addresses his comment about the use of "instance" dimension for "features". Open that commit to see what was changed. That commit is also now part of the total PR diff visible at the "files changed" link at the top of this page.

After all this conversation, it seems that the pull request review feature is really what you are looking for. When you go to make a comment on a line there is an option to "start a review".
:+1:

OK, thanks for all this discussion. I'll try to find time to do this in the next week or so. I'm a bit anxious that this might seem more complicated and consequently off-putting than trac would be for the average CF user who wants to propose a change to the convention text. But I need to try it to see how it works.

Dear Dave
At last I have managed to make time to work on your proposal. I'm sorry it took me so long. It turned out that I had a lot of suggestions to make, and I have edited it extensively. However, these changes are nearly all about presentation, clarity, logical order (to my mind - you may disagree) and removing repetition. It didn't seem possible to do this with GitHub comments so instead I have edited your AsciiDoc text. AsciiDoc is not the same as MarkDown, so GitHub can't render it correctly, but it's not very different.
In my next posting to the issue, I paste in my edited AsciiDoc, with the sole extra change of using four tildes to delimit verbatim code, as required by MarkDown. The AsciiDoc markup for headings doesn't work. Having gone through this exercise, I wonder whether in future it would be best to develop conventions changes in GitHub in MarkDown format, and convert it to AsciiDoc when it's agreed. There is at least one program (pandoc) which is said to be able to do this. Or could we store the conventions document in MarkDown rather than AsciiDoc? I assume that MarkDown doesn't have all the required facilities, since AsciiDoc appears to be more powerful.
In my third posting to the issue, I've given my revised text without the deletions and comments. I derived it this with sed '/^>/d; s/~~[^~][^~]*~~//g'.
Working on this did raise some substantive issues about your proposal, which I've commented on in the text. They are (a) I don't like the duplication of first and last points in a polygon. (b) Is it really necessary to use opposite orders for exterior and interior polygons? (c) It seems better to me to regard a polygon with holes as a multipolygon. (d) If there is only a single geometry, the node count variable could be omitted. (e) I have two concerns with the part about CRSs.
Best wishes
Jonathan

Could we call the new features just "geometries"? What does "spatial" add, since geometries are usually in space? Perhaps some other adjective would be more informative.

[[spatial-geometries, Section 7.5, "Spatial Geometries"]]
=== Spatial Geometries

The difference of this new convention from bounds is that there can be a different number of nodes for each of the elements of the axis, hence my suggestions  the next para. The ref to 9.3.3 should be a named reference, I suppose. I rearranged the first sentence, and moved and modified the first two sentences from your second para into this one, as definitions. I moved the watershed example here from a later para.

For many geospatial applications, data values (e.g., the average monthly rainfall in the UK) are associated with a [spatial] geometry, which is a spatial representation of a real-world feature, for instance a time-series of areal average precipitation over a watershed. Polygonal cells with an arbitrary number of multiple vertices are geometries, and can be described using <>, but in that case every cell must have the same number of vertices. In contrast, each [spatial] geometry associated with a given data variable may have a different number of nodes, the geometries may be lines (as alternatives to points and polygons), and they may be spatial geometries contain an arbitrary number of nodes for each geometry and include line and multipart geometries i.e. including several disjoint parts (e.g., the different islands of the UK). Other geometry types exist and may be introduced in a later version of the specification CF convention.  The approach described here specifies how to encode such geometries following the pattern in 9.3.3 Contiguous ragged array representation and attach them to variables in a way that is consistent with the cell bounds approach.

I've put all the next para in other places, except that I don't think we need to list the types here, because that's done when the geometry attribute is defined.

A geometry is usually thought to be a spatial representation of a real-world feature. It can be disjoint, having multiple parts. Geometry types are limited to point, multipoint, line, multiline, polygon and multipolygon types. Similar to other geospatial data formats, geometries are encoded as ordered sets of geospatial nodes. The connection between nodes is assumed to be linear in the coordinate reference system the nodes are defined in. Parametric geometries or otherwise curved features may be supported in the future.
All geometries are made up of one or more nodes. As in other geospatial data formats, geometries are encoded as ordered sets of nodes. The connection between nodes is assumed to be linear in the coordinate reference system the nodes are defined in. (Parametric geometries or otherwise curved features may be supported in the future.)

It is redundant to supply the last point of a polygon. If you replicate the last point, there's the possibility for inconsistency, and it needs to be checked; if the replicated point is not supplied, no mistake can be made! Hence my change here. It also saves a small amount of space.

The geometry type specifies the set of topological assumptions to be applied to relate the nodes. Multipoint and line geometries are nearly the same except nodes are interpreted as being connected for lines. Lines and polygons are also nearly the same except the first and last nodes must be identical for polygons that a polygon is completed by connecting the last node back to the first. Polygons that have holes, such as waterbodies in a land unit, are encoded as a collection of polygon ring parts, each identified as exterior or interior polygons. Multipart geometries, such as multiple lines representing the same river or multiple islands representing the same jurisdiction, are encoded as collections of un-connected points, lines, or polygons that are logically grouped into a single geometry.

I think we should omit the first sentence of this para because we don't need to explain the history of the convention. I have moved the idea of the second sentence to the end of the paragraph. I don't think your timeseries example belongs in Appendix E: it should be in this subsection, with the other example. It could also be referred to somewhere in Sect 9.

While this geometry encoding is applicable to any variable that shares a dimension with a set of geometry, the application it was originally designed for requires that geometry be joined to the instance dimension of a Discrete Sampling Geometry timeSeries featureType. The instance dimension, in the more general case of features, specifies the number of features in the collection and is also referred to as the feature dimension. In this case, any Any data variable can be given a geometry attribute that is to be interpreted as indicates the representative geometry for the quantity held in the variable. An example of this is areal average precipitation over a watershed. An example of line geometry with time series data is given in <>. One of the dimensions of the data variable must be the number of geometries to which the data applies. If the data variable has a discrete sampling geometry, the number of geometries is the instance dimension (Section 9.2).

I think the subheading is probably not needed.

==== Geometry Variables and Attributes

I suggest omitting the last two sentences because it seems easy enough to read if we just introduce the attributes in turn.

A set of geometries can be added to a file by inserting all required data variables and a A geometry container variable that acts as a container for attributes that describe a set of geometries. The geometry attribute of the data variable contains containing the name of a geometry container variablecan be added to any variable that shares the feature dimension with the geometries. The geometry container variable must hold geometry_type and node_coordinates attributes. Depending on the geometry_type, the geometry container may also need to contain a node_count, part_node_count, and interior_ring attribute. These attributes are described in detail below.

In the next two paras, we don't need to repeat the requirements that these attributes must be supplied. I avoided naming x,y,z because that is obvious from the next sentence, which was a later para. I moved it here and shortened it by removing repetition. I put "blank-delimited" because that's the phrase we normally use.

The geometry_type attribute must be carried by a geometry container variable and indicates the type of geometry present. Its allowable values are: point, multipoint, line, multiline, polygon, multipolygon.
The node_coordinates attribute must be carried by a geometry container variable and contains the space blank delimited names of the x and y (and z) variables that contain geometry node coordinates (one variable for each spatial dimension).

Is it really necessary to use opposite orders for exterior and interior? It trikes me that the convention would be simpler and hence more reliable if they were all anticlockwise. You know which are interior ones from the interior_ring attribute and so can reverse them if necessary when doing calculations.

The variables that contain geometry node coordinate variables data, indicated by the node_coordinates attribute on the geometry container variable, are also identifiable through the use of a required must each have a cf_role attribute, whose allowable values are geometry_x_node, geometry_y_node, and geometry_z_node. The geometry node coordinate variables must all have the same single dimension, which is the total number of nodes in all the geometries. The nodes must be stored consecutively for each geometry and and in the order of the geometries, and within each multipart geometry the nodes must be stored consecutively for each part and in the order of the parts. Polygon exterior rings must be put in anticlockwise order (viewed from above) and polygon interior rings in clockwise order. They are put in opposite orders to facilitate calculation of area.

If there is only a single geometry, the node count variable could be omitted, couldn't it, because all the nodes must belong to that geometry. That would be a simplification.

For all geometry types except point, in which each geometry contains a single node, the geometry container variable must have a node_count attribute that contains the name of a variable indicating the count of nodes per geometry. Note that For a multipart geometry, the node count may span is the total for all the multiple geometry parts. The single dimension of the node count variable should be the number of geometries.

I gather from the next para that you regard a single polygon with holes as a polygon still. Isn't it simpler to treat it as a multipolygon? That means the requirements for attributes are simpler, and in terms of processing the data it seems natural to me. I have changed it to match that assumption, but you may well have counter-arguments.

For multiline and multipolygon geometries (and including polygons with holes), the geometry container variable must have a part_node_count attribute that contains the name of a variable indicating the count of nodes per geometry part. Note that because multipoint geometries always have a single node per part, the part_node_count is not required. The single dimension of the part node count variable should equal the total number of parts in all the geometries.

See last para concerning polygons with holes.

For polygon and multipolygon geometries with holes, the geometry container variable must have an interior_ring attribute that contains the name of a variable that indicates if the polygon parts are interior rings (i.e., holes) or not. The variable indicated by the interior_ring attribute This interior ring variable should contain the value 0 to indicate an exterior ring polygon and 1 to indicate an interior ring polygon. Note that single part polygons can have interior rings; multipart polygons are distinct in that they have more than one exterior ring. The single dimension of the interior ring variable should be the same dimension as that of the part node count variable.

I moved this final para to an earlier point

The variables that contain geometry node coordinate data, indicated by the node_coordinates attribute on the geometry container variable, are also identifiable through the use of a required cf_role attribute. Allowable values are geometry_x_node, geometry_y_node, and geometry_z_node.

I don't think the following section is needed. See below.

==== Encoding Geometries

The first sentence of this is "history", and I think the point is better made earlier (see above). I have put the rest of the requirements at earlier points where these variables are introduced.

Geometry encoding follows a similar pattern to the contiguous ragged array approach in 9.3.3 Contiguous ragged array representation with some modification to suit the spatial geometry use case rather than observational time series. All spatial data are encoded in the variables indicated by the node_coordinates and appropriate cf_role attribute. These node variables should be one dimensional and total number of nodes long. There are three one dimensional variables that are used to break up and interpret the node variabes: node_count, part_node_count, and interior_ring.

Some of the next two paragraphs is repetition, and I have put the other requirements at earlier points.

For geometry types requiring a node_count attribute, the node count variable should be the number of geometries long and indicate the number of nodes per geometry. For geometry types requireing a part_node_count attribute, the part node count variable should be the number of geometry parts long and indicate the number of nodes per geometry part. For geometry types requireing an interior_ring attribute, the interior ring variable should be the number of geometry parts long and contain 0s and 1s to indicate exterior or interior.
The ecosystem of polygon specifications and software implementations of those specifications varies in how polygons are encoded. Nodes within a polygon exterior or interior ring are typically encoded in opposite clockwise or anticlockwise direction around the polygon. This is important for operations such as caluclating area. CF requires that outer rings be encoded in anticlockwise order and interior rings be encoded in clockwise order. CF also requires that the first and last node in a polygon be identical to ensure polygon rings are complete.

I have two concerns with the next paragraph. First, why should a grid mapping be required in the case of geometries when it's not otherwise? I'm sure that in many real-world applications it is desirable and should be included, but it's possible this convention could be used with model or other data in which a grid mapping isn't needed or useful. (NB a grid mapping doesn't exactly correspond to a CRS. Parts of a CRS reside in other parts of CF metadata.) I would prefer a recommendation to include a grid_mapping for real-world data, rather than a requirement. Second, if a grid mapping is needed for a geometry when there is no data variable, why not put a grid_mapping attribute on the geometry container variable? As far as I can see, the proposed crs attribute is the same thing. But why do we need this anyway, since there is always a data variable in the file?

A coordinate reference system (CRS) (referred to as a grid mapping elsewhere in the CF convention) is strictly required for geometries. The normal CF practice, of attaching a grid_mapping attribute--containing the name of a CRS container variable--to a data variable, can be used and the grid_mapping CRS should be assumed to apply to the geometry. However, the normal grid_mapping, which typically applies to auxiliary coordinate variables and remains optional for use with geometries, can be overridden by attaching a crs attribute that contains the name of a CRS container variable to the geometry container variable. If a grid_mapping is not present on a data variable linked to geometry, a crs attribute is required.
[[complete-multipolygon-example]]
[caption="Example 7.14. "]
.A multipolygon with holes
This example demonstrates the use of all potential attributes and variables for encoding geometries.
dimensions:
  node = 25 ;
  feature = 1 ;
  part = 6 ;
variables:
  double x(node) ;
    x:units = "degrees_east" ;
    x:standard_name = "longitude" ;
    x:cf_role = "geometry_x_node" ;
  double y(node) ;
    y:units = "degrees_north" ;
    y:standard_name = "latitude" ;
    y:cf_role = "geometry_y_node" ;
  float geometry_container ;
    geometry_container:geometry_type = "multipolygon" ;
    geometry_container:node_count = "node_count" ;
    geometry_container:node_coordinates = "x y" ;
    geometry_container:crs = "crs" ; // I'm doubtful about this - see above
    geometry_container:part_node_count = "part_node_count" ;
    geometry_container:interior_ring = "interior_ring" ;
  int node_count(feature) ; // could be omitted since feature=1 - see above
    node_count:long_name = "count of coordinates in each feature geometry" ;
  int part_node_count(part) ;
    part_node_count:long_name = "count of nodes in each geometry part" ;
  int interior_ring(part) ;
    interior_ring:long_name = "type of each geometry part" ;
  float crs ;
    crs:grid_mapping_name = "latitude_longitude" ;
    crs:semi_major_axis = 6378137. ;
    crs:inverse_flattening =<PHONE_NUMBER>63 ;
    crs:longitude_of_prime_meridian = 0. ;
// global attributes:
  :Conventions = "CF-1.8" ;
data:
 x = 0, 20, 20, 0, 0, 1, 10, 19, 1, 5, 7, 9, 5, 11, 13, 15, 11, 5, 9, 7, 5,
    11, 15, 13, 11 ;
 y = 0, 0, 20, 20, 0, 1, 5, 1, 1, 15, 19, 15, 15, 15, 19, 15, 15, 25, 25, 29,
    25, 25, 25, 29, 25 ;
 geometry_container = 0. ;
 node_count = 25 ;
 part_node_count = 5, 4, 4, 4, 4, 4 ;
 interior_ring = 0, 1, 1, 1, 0, 0 ;
 crs = 0. ;

I think your timeseries example should appear here (not in Appendix E).
====

[[spatial-geometries, Section 7.5, "Spatial Geometries"]]
=== Spatial Geometries
For many geospatial applications, data values (e.g., the average monthly rainfall in the UK) are associated with a [spatial] geometry, which is a spatial representation of a real-world feature, for instance a time-series of areal average precipitation over a watershed. Polygonal cells with an arbitrary number of multiple vertices are geometries, and can be described using <>, but in that case every cell must have the same number of vertices. In contrast, each [spatial] geometry associated with a given data variable may have a different number of nodes, the geometries may be lines (as alternatives to points and polygons), and they may be multipart i.e. including several disjoint parts (e.g., the different islands of the UK). Other geometry types exist and may be introduced in a later version of the  CF convention.  The approach described here specifies how to encode such geometries following the pattern in 9.3.3 Contiguous ragged array representation and attach them to variables in a way that is consistent with the cell bounds approach.
All geometries are made up of one or more nodes. As in other geospatial data formats, geometries are encoded as ordered sets of nodes. The connection between nodes is assumed to be linear in the coordinate reference system the nodes are defined in. (Parametric geometries or otherwise curved features may be supported in the future.)
The geometry type specifies the set of topological assumptions to be applied to relate the nodes. Multipoint and line geometries are nearly the same except nodes are interpreted as being connected for lines. Lines and polygons are also nearly the same except  that a polygon is completed by connecting the last node back to the first. Polygons that have holes, such as waterbodies in a land unit, are encoded as a collection of polygon ring parts, each identified as exterior or interior polygons. Multipart geometries, such as multiple lines representing the same river or multiple islands representing the same jurisdiction, are encoded as collections of un-connected points, lines, or polygons that are logically grouped into a single geometry.
Any data variable can be given a geometry attribute that  indicates the  geometry for the quantity held in the variable.  One of the dimensions of the data variable must be the number of geometries to which the data applies. If the data variable has a discrete sampling geometry, the number of geometries is the instance dimension (Section 9.2).
==== Geometry Variables and Attributes
A geometry container variable  acts as a container for attributes that describe a set of geometries. The geometry attribute of the data variable contains  the name of a geometry container variable. The geometry container variable must hold geometry_type and node_coordinates attributes.
The geometry_type attribute  indicates the type of geometry present. Its allowable values are: point, multipoint, line, multiline, polygon, multipolygon.
The node_coordinates attribute  contains the  blank delimited names of the  variables that contain geometry node coordinates (one variable for each spatial dimension).
The  geometry node coordinate variables must each have a cf_role attribute, whose allowable values are geometry_x_node, geometry_y_node, and geometry_z_node. The geometry node coordinate variables must all have the same single dimension, which is the total number of nodes in all the geometries. The nodes must be stored consecutively for each geometry and and in the order of the geometries, and within each multipart geometry the nodes must be stored consecutively for each part and in the order of the parts. Polygon exterior rings must be put in anticlockwise order (viewed from above) and polygon interior rings in clockwise order. They are put in opposite orders to facilitate calculation of area.
For all geometry types except point, in which each geometry contains a single node, the geometry container variable must have a node_count attribute that contains the name of a variable indicating the count of nodes per geometry.  For a multipart geometry, the node count  is the total for all the  parts. The single dimension of the node count variable should be the number of geometries.
For multiline and multipolygon geometries ( including polygons with holes), the geometry container variable must have a part_node_count attribute that contains the name of a variable indicating the count of nodes per geometry part. Note that because multipoint geometries always have a single node per part, the part_node_count is not required. The single dimension of the part node count variable should equal the total number of parts in all the geometries.
For  multipolygon geometries with holes, the geometry container variable must have an interior_ring attribute that contains the name of a variable that indicates if the polygon parts are interior rings (i.e., holes) or not.  This interior ring variable should contain the value 0 to indicate an exterior ring polygon and 1 to indicate an interior ring polygon.  The single dimension of the interior ring variable should be the same dimension as that of the part node count variable.
==== Encoding Geometries
[[complete-multipolygon-example]]
[caption="Example 7.14. "]
.A multipolygon with holes
This example demonstrates the use of all potential attributes and variables for encoding geometries.
dimensions:
  node = 25 ;
  feature = 1 ;
  part = 6 ;
variables:
  double x(node) ;
    x:units = "degrees_east" ;
    x:standard_name = "longitude" ;
    x:cf_role = "geometry_x_node" ;
  double y(node) ;
    y:units = "degrees_north" ;
    y:standard_name = "latitude" ;
    y:cf_role = "geometry_y_node" ;
  float geometry_container ;
    geometry_container:geometry_type = "multipolygon" ;
    geometry_container:node_count = "node_count" ;
    geometry_container:node_coordinates = "x y" ;
    geometry_container:crs = "crs" ; // I'm doubtful about this - see above
    geometry_container:part_node_count = "part_node_count" ;
    geometry_container:interior_ring = "interior_ring" ;
  int node_count(feature) ; // could be omitted since feature=1 - see above
    node_count:long_name = "count of coordinates in each feature geometry" ;
  int part_node_count(part) ;
    part_node_count:long_name = "count of nodes in each geometry part" ;
  int interior_ring(part) ;
    interior_ring:long_name = "type of each geometry part" ;
  float crs ;
    crs:grid_mapping_name = "latitude_longitude" ;
    crs:semi_major_axis = 6378137. ;
    crs:inverse_flattening =<PHONE_NUMBER>63 ;
    crs:longitude_of_prime_meridian = 0. ;
// global attributes:
  :Conventions = "CF-1.8" ;
data:
 x = 0, 20, 20, 0, 0, 1, 10, 19, 1, 5, 7, 9, 5, 11, 13, 15, 11, 5, 9, 7, 5,
    11, 15, 13, 11 ;
 y = 0, 0, 20, 20, 0, 1, 5, 1, 1, 15, 19, 15, 15, 15, 19, 15, 15, 25, 25, 29,
    25, 25, 25, 29, 25 ;
 geometry_container = 0. ;
 node_count = 25 ;
 part_node_count = 5, 4, 4, 4, 4, 4 ;
 interior_ring = 0, 1, 1, 1, 0, 0 ;
 crs = 0. ;

====

On a phone, so only a few comments for now.
I have edited your AsciiDoc text. AsciiDoc is not the same as MarkDown, so
GitHub can't render it correctly, but it's not very different.
I'm pretty sure gitHub can render asciidoc.
But in any case, if you want to know exactly how it's going to render, you
need to use the same renderer and format. So there is really no point in
putting markdown in here.
If you want to work on the content and structure, just do it in plain text
-- that's the whole point of these plain text-based markup formats.
In my next posting to the issue, I paste in my edited AsciiDoc,
If you want to make major changes to the entire doc, rather than commenting
on bits, you're really better off forking the repo and making the changes
and doing a pull request. That's exactly what the gitHub workflow is for,
-CHB

Comment on the real content.,.
Working on this did raise some substantive issues about your proposal,
which I've commented on in the text. They are (a) I don't like the
duplication of first and last points in a polygon.
This is legacy from older geo formats:
"You’ll also notice that the first coordinate is repeated at the end of
each ring. There’s no particular reason why this is necessary besides
GeoJSON’s heritage in older formats."
In some, repetition of the point distinguishes between a polygon and a
polyline. Not required here, but compatibility with other formats is s good
thing.
(b) Is it really necessary to use opposite orders for exterior and interior
polygons? (c) It seems better to me to regard a polygon with holes as a
multipolygon.
A polygon with holes is a Different Thing than multiple polygons.
For instance, what is the area of a polygon with a hole? Vs two polygons
that happen to overlap? And using winding order makes processing easier.
And again, compatibility with other formats.
(d) If there is only a single geometry, the node count variable could be
omitted.
It could be, but it makes it easier to write processing code if it's always
there.
-CHB

Dear Chris

I'm pretty sure gitHub can render asciidoc.

Maybe it can. How do you tell the issue that you're giving it AsciiDoc rather than MarkDown?

If you want to make major changes to the entire doc, rather than commenting on bits, you're really better off forking the repo and making the changes and doing a pull request. That's exactly what the gitHub workflow is for.

Yes, we discussed it above, and the conclusion was that I should comment on the pull request. However, I really do not think that's very human-friendly for editing text like this (not friendly to this human, anyway). To make things easy to appreciate, you want to see the inserted text, deleted text, and comments all in front of you. That is what I have tried to achieve above, using a bit of MarkDown. (I forgot to point out that I used italic for inserted text and strikethrough for deleted - hopefully obvious.) I think the workflow involving comments on pull requests would be appropriate just for correcting final errors or making minor insertions.

Duplication of first and last points in a polygon ... is legacy from older geo formats. ... compatibility with other formats is a good thing.

Yes, if it's free, but this has a cost (redundancy and possible inconsistency) so I'm not convinced. This is a different file format so if you're translating from another one you will have to do some work anyway. Deleting or inserting a duplicated point as part of the translation wouldn't be complicated.

A polygon with holes is a Different Thing than multiple polygons. For instance, what is the area of a polygon with a hole? Vs two polygons that happen to overlap?

I agree those are different things, but they're clearly distinguished by the interior and exterior ring marking. A polygon with a hole is a multipolygon where one is exterior and one interior. Two overlapping polygons are both exterior.

If there is only a single geometry, the node count variable could be omitted. It could be, but it makes it easier to write processing code if it's always there.

Maybe, but by that argument the part_node_count variable and the interior_ring variable should always be present too.
Best wishes
Jonathan

Dear Jonathan,
Thanks for getting back with us on this.
I think I've covered most of the comments with my personal disposition on them. This issue is getting a bit ungainly. I think that I will probably close it and move your comments to a google doc where we can actually do word by word diffing. (see my comment on this below) This level of review of the text is not supported by github, where we should probably just be vetting small changes and the actual asciidoc syntax. I'll wait to do this till tomorrow or the next day in case there's a real push to keep this review here...
GitHub Usage Response

I'm pretty sure gitHub can render asciidoc.

Comments only support markdown and the diff view of a pull request will never be "rendered". .adoc files do render in github. The best solution would probably to comment paragraph by paragraph in plain text in the diff view. I'll migrate your comments to that style as an example.

If you want to make major changes to the entire doc, rather than commenting on bits, you're really better off forking the repo and making the changes and doing a pull request. That's exactly what the gitHub workflow is for.

As I expected, for editing paragraphs of text, word-by-word diffing is desirable and it seems that the only way to get this is with some manual indication of what's been deleted and what's been inserted. (I suggested this above, but commenting line by line with this kind of thing as needed may be a good happy medium?)
Additionally, I think we may consider having contributions vetted in a more text-friendly system, such as google docs, or another more ad-hoc space then use this mechanism for review of the actual ascii doc syntax of agreed upon text. I've suggested it before, but this is a good example of how we are mixing purposes (content review and syntax review). I used google docs for a previous submission, would it be preferable to move this review there so we can use normal document track changes till the text is finalized?
Proposal Response .

Duplication of first and last points in a polygon ... is legacy from older geo formats. ... compatibility with other formats is a good thing.

I have the same stance as @ChrisBarker-NOAA on this. It's just a common way of being defensive in the data encoding. It is an extra bit of certainty that the linear ring is complete and wasn't corrupted on the way in or out of a file. I'm not attached, but the idea of multiple layers of certainty for protection against data corruption is a positive that you may not be thinking about? As another point of reference, the OGC Simple Features Access spec follows this convention as well.

A polygon with holes is a Different Thing than multiple polygons. For instance, what is the area of a polygon with a hole? Vs two polygons that happen to overlap?

I feel pretty strongly that we should NOT say that a polygon with a hole is a multipolygon.

A polygon is a thing with some area. You calculate the area of the outer ring and subtract the area of the inner rings. The shoelace formula relies on opposite ordering for this.
Being consistent with other geospatial formats is a big positive for adoptability and familiarity of a format.
This is also consistent with the standing OGC Simple Feature Access standard, which is the canonical reference for this kind of data at this point IMHO.

If there is only a single geometry, the node count variable could be omitted. It could be, but it makes it easier to write processing code if it's always there.

I have no problem allowing the node_count to be omitted.

I think we should omit the first sentence of this para because we don't need to explain the history of the convention.

I've seen the history of the convention described elsewhere (specifically in the DSG spec), and was following that. No problem removing the content there.

First, why should a grid mapping be required in the case of geometries when it's not otherwise?

As long as it can be required for data associated with real-world coordinates. I am uncomfortable allowing anyone to not-state their datum/projection assumptions when recording lat/lon or other real-world coordinates.

As far as I can see, the proposed crs attribute is the same thing. But why do we need this anyway, since there is always a data variable in the file?

While "crs" is the same thing, the grid_mapping variable name is problematic because we are not dealing with grids here. If it's objectionable, I'm happy to just use grid_mapping, but it could be confusing. Regarding putting a crs attribute on data variables, that has always confused me. Why not attach the spatial metadata to the spatial data directly, as we've suggested here - attaching a crs to the geometry container variable? It's indirect and just odd that the metadata goes on a data variable that references the spatial data. I have no problem following the existing convention, but it's a really odd artifact of CF that I've never really understood the logic for.
Best,

Dave

Dear Dave, Chris and Jonathan,
A few comments on the proposal
.
I haven't though about github usage
 yet
, but I will try a few text edit suggestions based on my comments below.
I don't think that we need to have the cf_role attribute on the node
variables. Their role is well defined by them being named
geometry_container:node_coordinates attribute, in just the same way that
auxiliary coordinates are identified from the coordinate attribute of a
data variable. The use of cf_role in DSGs is different, I think - in that
case there is nothing else to say how it relates to the data variable.
It would be very useful to include a data variable with a geometry
attribute in example 7.14 (A multipolygon, one part of which has holes).
One thing that is not clear to me is how you would specify multiple
geometries along a dimension. For example, if I have a data variable with
shape 12x3 that contains three equal length timeseries of countrywide
averages for the UK, Netherlands and France, how would I attach the three
geometries defining the three countries?

Duplication of first and last points in a polygon ... is legacy from older
geo formats. ... compatibility with other formats is a good thing.
I have the same stance as @ChrisBarker-NOAA
https://github.com/chrisbarker-noaa on this. It's just a common way of
being defensive in the data encoding. It is an extra bit of certainty that
the linear ring is complete and wasn't corrupted on the way in or out of a
file. I'm not attached, but the idea of multiple layers of certainty for
protection against data corruption is a positive that you may not be
thinking about? As another point of reference, the OGC Simple Features
Access https://en.wikipedia.org/wiki/Shoelace_formula spec follows this
convention as well.

I'm not keen on the duplicating the start/end node. We don't duplicate
points for other cyclic entities (e.g. global longitude coordinates) and if
the repeated nodes were there, I'd feel duty bound to check them for
equality, which is never nice with floats.
A polygon with holes is a Different Thing than multiple polygons. For

instance, what is the area of a polygon with a hole? Vs two polygons that
happen to overlap?
I feel pretty strongly that we should NOT say that a polygon with a hole
is a multipolygon.

A polygon is a thing with some area. You calculate the area of the
outer ring and subtract the area of the inner rings. The shoelace
formula https://en.wikipedia.org/wiki/Shoelace_formula relies on
opposite ordering for this.
Being consistent with other geospatial formats is a big positive
for adoptability and familiarity of a format.
This is also consistent with the standing OGC Simple Feature Access
https://en.wikipedia.org/wiki/Shoelace_formula standard, which is
the canonical reference for this kind of data at this point IMHO.

I'm happy with this definition - we should have some text that more
precisely defines a polygon (happy to initialise).

First, why should a grid mapping be required in the case of geometries
when it's not otherwise?
As long as it can be required for data associated with real-world
coordinates. I am uncomfortable allowing anyone to not-state their
datum/projection assumptions when recording lat/lon or other real-world
coordinates.
As far as I can see, the proposed crs attribute is the same thing. But why
do we need this anyway, since there is always a data variable in the file?
While "crs" is the same thing, the grid_mapping variable name is
problematic because we are not dealing with grids here. If it's
objectionable, I'm happy to just use grid_mapping, but it could be
confusing. Regarding putting a crs attribute on data variables, that has
always confused me. Why not attach the spatial metadata to the spatial data
directly, as we've suggested here - attaching a crs to the geometry
container variable? It's indirect and just odd that the metadata goes on a
data variable that references the spatial data. I have no problem following
the existing convention, but it's a really odd artifact of CF that I've
never really understood the logic for.

I don't favour a special rule for one type of bounds over another, so I
would not like to see the crs as mandatory, nor attachable to the geometry
container. Viewing the geometries as bounds, I think that we do have grid
... which raises the issue that a geometry is bounds without an associated
coordinate value. This is new in CF. Is it desirable? I can see use for a
representative location that is useful (e.g. the average of the nodes' lats
and lons).
All the best,
David
--
David Hassell
National Centre for Atmospheric Science
Department of Meteorology, University of Reading,
Earley Gate, PO Box 243, Reading RG6 6BB
Tel: +44<PHONE_NUMBER>
http://www.met.reading.ac.uk/

Dear Dave and David
Thanks for your comments.
I agree with David that a multipolygon example would be useful.
Regarding duplicating points, another argument is for consistency within, as David says. CF can already describe polygonal bounds for a cell. In that case, the points are ordered anticlockwise (like a polygon in the new convention) and the first and last point are joined, with no repeated point. I think it's more important for CF to be consistent with CF than with other conventions. CF generally avoids redundancy.
I could accept that a polygon with holes is different from a multipolygon, but I'd appreciate some further clarification.  So are two polygons. Thus, structurally, a polygon with holes is like a multipolygon.
Dave says "I am uncomfortable allowing anyone to not-state their datum/projection assumptions when recording lat/lon or other real-world coordinates." However, that is the situation in CF. Even for the real world, it is optional to state these assumptions, because they are not always relevant to the dataset in question. It wouldn't be consistent to take a different view in this particular part of the convention.
Consistency is also the reason for using the attribute for grid_mapping, not introduce a new attribute with exactly the same function. Also CRS isn't an accurate description of the grid mapping variable, which contains only part of the CRS. David has thought about this in most detail.
Best wishes
Jonathan

Dear Jonathan and David,
I can accept the consistency within CF argument, and don't think it's a big enough problem to allow polygons to not self-close.
Will attempt to petter define geometry such that a single-part geometry can have holes.
The CRS issue is an old horse I've been riding for a long time. My issue is that:

Even for the real world, it is optional to state these assumptions, because they are not always relevant to the dataset in question.

is simply not true. There are no real world coordinates that do not require a statement of the reference system they use. My stance on the issue is that, if a grid_mapping is to be optional, CF should have a default, such as WGS84. In addition, if people are using a coordinate system that has no connection to the earth, then that should be explicit by defining a grid_mapping with grid_mapping_name "non_earth_coordinates" or something. However, this is not the issue at hand and I'm fine following the existing convention on this front for now.
Still curious about my suggestion re: modifying text in google docs or another editor designed for collaborative writing (not coding)... thoughts?
Best,

Dave

I can accept the consistency within CF argument, and don't think it's a big enough problem to allow polygons to not self-close.

I agree here -- I think we should think of this as a way to represent OGC-standard data in netCDF in a CF-compatible way, rather than "A way to specify geometries in CF"
I have enough trouble getting GIS folks to deal with netcdf as it is....

Will attempt to petter define geometry such that a single-part geometry can have holes.

Good idea, and good to provide a Multipolygon (and polygons with holes) example.
And if we do keep the "OGC compatible" emphasis, then we can point folks to OGC standards for reference.

is simply not true. There are no real world coordinates that do not require a statement of the reference system they use. My stance on the issue is that, if a grid_mapping is to be optional, CF should have a default, such as WGS84.

I agree -- It's probably too late to require that it be specified, but using WGS84 as an official default is a good way to capture (most) use-cases.
There is no such thinking as "no reference system", there is only sloppiness. :-)
In addition, if people are using a coordinate system that has no connection to the earth, then that should be explicit by defining a grid_mapping with grid_mapping_name "non_earth_coordinates" or something.
and an "unknown" DRS may be needed for when people are pulling data from another source that was already sloppy...

Still curious about my suggestion re: modifying text in google docs or another editor designed for collaborative writing (not coding)... thoughts?

Google docs is a great way to collaborate on a document -- much better than passing MSWord files around, etc.
But one of the great things about asciidoc (and it's like) is that they're plain text, just like code, and thus tools designed for code work will with it -- git, github, diff, etc....
So the gitHub workflow is actually a pretty good one for this:

If you want to comment on specific bits of a doc, put line by line comments in the Pull Request.

If you want to make large changes to a page, then fork (or branch) the repo and make your changes. Then do a PR -- In the PR, the changes can be seen in diff view, which is a good tool for review.

Also, it looks like asciidoc supported comments:
http://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#comments
so you can add comments and/or comment out chunks of text, rather than deleting them.
All that being said, if there are 2-3 people that want to collaborate on it in Google docs, then bring it back into gitHub, go for it.
What will be lost is the process -- but that probably doesn't matter.

The issue with comments on documents (as opposed to code) is that documents have lines of text that are much more than the typical 60-80 characters that's typical of code. GitHub is optimized for syntax and short-line commenting. It works well if comments are conceptual or like, "can you reword this?", but not so good for detailed editing such as Jonathan's latest. For now, I'll close this PR, reconcile Jonathan's suggestions into a new version of the doc and open a new PR that references this one. This comment string is enough to rinse and repeat with a reference back here for old conversation.

Hi Dave,
Does this closing this mean that discussion is currently not possible, and will opening a new pull request  mean that the discussion will ultimately be split across different threads?
Thanks,
David

@dblodgett-usgs wrote:

The issue with comments on documents (as opposed to code) is that documents have lines of text that are much more than the typical 60-80 characters that's typical of code. GitHub is optimized for syntax and short-line commenting.

True, but:
if you have a document that has entire paragraphs as single line,s it makes it easy to comment on a paragraph, so not so bad, really.
Ans ascidoc and other markups don't assign significance to a single line break, so you can, (and should) use moderate sized lines anyway.
But this by no means needs to be resolved on this issue, and at the end of the day any method that works for the folks taking part is the one to go with.

@davidhassell:
closing the issue means it shows up as closed when you search for it. But you can still add to it, and what you add will be captured for posterity (like this one...)
Opening a new PR will create a new discussion thread for that PR -- so yes, the discussion will be split, but I think that's probably a good thing -- folks can look here for the history, but the new PR will have discussion on what will hopefully be a close to final proposal, having included all this discussion.


Add usage instructions into README.md

Also polish up the usage if there's usability issues.

https://github.com/myyk/git-democracy/pull/34
https://github.com/myyk/git-democracy/pull/35


Restrict S3 permissions to readonly for consumption role dataset sharing policy

… read only for consumption role
Feature or Bugfix

Bugfix

Detail

This PR will address the issue - 1226

Relates

Security
Please answer the questions below briefly where applicable, or write N/A. Based on
OWASP 10.

Does this PR introduce or modify any input fields or queries - this includes
fetching data from storage outside the application (e.g. a database, an S3 bucket)? N/A

Is the input sanitized? N/A
What precautions are you taking before deserializing the data you consume? N/A
Is injection prevented by parametrizing queries? N/A
Have you ensured no eval or similar functions are used? N/A

Does this PR introduce any functionality or component that requires authorization? N/A

How have you ensured it respects the existing AuthN/AuthZ mechanisms? N/A
Are you logging failed auth attempts? N/A

Are you using or adding any cryptographic features? N/A

Do you use a standard proven implementations? Yes
Are the used keys controlled by the customer? Where are they stored? N/A

Are you introducing any new policies/roles/users? No

Have you used the least-privilege principle? How? Yes, restricted the s3:* permissions to s3 readonly permissions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Checked.
Permissions are given according to PR.
User can list the objects in the bucket and download them.
When item is revoked, permissions are deleted correctly

Hi @SofiaSazonova, thank you for approving the PR.  I added the permissions to mimic the S3ReadOnly Managed Role. Should i drill this down further?


[Doc Issue][bot-sso-chat]

Bot SSO is available in only in 1:1 chat.

Acrolinx Scorecards
We currently enforce a minimum score of 80.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology:

Article
Score
Issues
Scorecard
Processed

msteams-platform/bots/how-to/authentication/bot-sso-overview.md
94
6
link
:white_check_mark:

More info about Acrolinx
Use the Acrolinx extension, or sidebar, in Visual Studio Code to check spelling, grammar, style, tone, clarity, and key terminology when you're creating or updating content. For more information, see Use the Visual Studio Code extension to run Acrolinx locally.

Learn Build status updates of commit 72ade98:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

msteams-platform/bots/how-to/authentication/bot-sso-overview.md
:white_check_mark:Succeeded
View

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching the learn.microsoft.com contributor guidesPost your question in the Learn support channel

Acrolinx Scorecards
We currently enforce a minimum score of 80.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology:

Article
Score
Issues
Scorecard
Processed

msteams-platform/bots/how-to/authentication/bot-sso-overview.md
94
6
link
:white_check_mark:

More info about Acrolinx
Use the Acrolinx extension, or sidebar, in Visual Studio Code to check spelling, grammar, style, tone, clarity, and key terminology when you're creating or updating content. For more information, see Use the Visual Studio Code extension to run Acrolinx locally.

Learn Build status updates of commit e1ee78c:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

msteams-platform/bots/how-to/authentication/bot-sso-overview.md
:white_check_mark:Succeeded
View

For more details, please refer to the build report.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
For any questions, please:Try searching the learn.microsoft.com contributor guidesPost your question in the Learn support channel


NetBSD support

This is a tracking issue for NetBSD support.

Blocked by the https://github.com/ebarnard/rust-plist/pull/34

FWIW the 0.4.2 release of rust-plist has the change referenced above

FWIW the 0.4.2 release of rust-plist has the change referenced above

Thanks for heads up, @rtyler! I tried to tackle this issue with NetBSD installed on a VirtualBox, but it was not able to emulate battery for some reasons, so I postponed it for a while.

Thanks for heads up, @rtyler! I tried to tackle this issue with NetBSD installed on a VirtualBox, but it was not able to emulate battery for some reasons, so I postponed it for a while.

I have a NetBSD-based Pinebook Pro, if you have some pointers, I could take a look at wiring this up in my spurious free time :smile_cat:

I have a NetBSD-based Pinebook Pro, if you have some pointers, I could take a look at wiring this up in my spurious free time :smile_cat:

@rtyler that would be really great, thanks!
I know nothing about NetBSD, so it would be great to start with figuring out what it can provide to us, looks like envstat tool should help us to do that?
Seems like Golang implementation uses ioctl to read the same information from /dev/sysmon fd with ENVSYS_GETDICTIONARY ioctl type (?) and it returns back proplist with whatever data there is.
When we will know what data we can operate with, we will need to check if it will be enough to fill Battery struct with it; if it is the case - everything is great, otherwise we might need to change public API first to suit NetBSD changes (I can work on that).

A friend complained that this crate does not support NetBSD, so I took a quick look at it. Adding 'target_os = "netbsd"' to all "freebsd" cases makes it compile, but the 5 tests fail. So I guess that is not sufficient :)
(NetBSD also uses acpica (same as FreeBSD) that's why I thought this might be enough.)
Then I found this issue.
NetBSD's envstat source code

The general framework for system monitoring and power management on NetBSD is sysmon (https://man.netbsd.org/sysmon.4) and in particular for environmental sensors, sysmon_envsys (documented here: https://man.netbsd.org/sysmon_envsys.9). I hope this is sufficient information!

@svartalf What you describe (read /dev/sysmon) is exactly what envstat does too. Caveat: by default, only root may read from the sysmon device, so there should be some fallback (e.g. pretend there is no battery?).

Another piece to take a look at could be e.g. https://github.com/NetBSD/pkgsrc/tree/trunk/sysutils/xbattbar which in the patches/ sub-directory contains a patch for adapting this to NetBSD.

Here's some sample code that gives a plist:
use std::error::Error;
use std::fs::File;
use std::os::unix::io::AsRawFd;
use nix::ioctl_readwrite;
use std::ffi::c_void;
use std::mem::MaybeUninit;
use plist::Value;
use std::slice::from_raw_parts;
use std::io::Cursor;

#[allow(non_camel_case_types)]
#[repr(C)]
#[derive(Debug)]
pub struct plistref {
        pref_plist: *mut c_void,               /* plist data */
        pref_len: usize,                /* total length of plist data */
}

ioctl_readwrite!(envsys_getdictionary, b'E', 0, plistref);

fn main() {
    match detect_sensors() {
        Ok(_) => (),
        Err(err) => println!("error: {}", err),
    }
}

fn detect_sensors() -> Result <(), Box<dyn Error>> {
    let envsys = File::open("/dev/sysmon")?;
    let mut dict = MaybeUninit::<plistref>::uninit();
    let _res = unsafe { envsys_getdictionary(envsys.as_raw_fd(), dict.as_mut_ptr()) };
    let dict = unsafe { dict.assume_init() };
    let u8slice: &[u8] = unsafe { from_raw_parts(dict.pref_plist as *const u8, dict.pref_len) };
    let cursor = Cursor::new(u8slice);
    let value = Value::from_reader(cursor)?;
    println!("{:?}", value);
    Ok(())
}

On one of my systems that prints:
Dictionary({"amdzentemp0": Array([Dictionary({"cur-value": Integer(314900000), "description": String("cpu0 temperature"), "index": String("sensor0"), "monitoring-state-refresh-event": Boolean(true), "monitoring-supported": Boolean(true), "state": String("valid"), "type": String("Temperature")}), Dictionary({"cur-value": Integer(313150000), "description": String("cpu0 ccd0 temperature"), "index": String("sensor1"), "monitoring-state-refresh-event": Boolean(true), "monitoring-supported": Boolean(true), "state": String("valid"), "type": String("Temperature")}), Dictionary({"device-properties": Dictionary({"device-class": String("other"), "refresh-timeout": Integer(30)})})]), "amdzentemp1": Array([Dictionary({"cur-value": Integer(311775000), "description": String("cpu1 temperature"), "index": String("sensor0"), "monitoring-state-refresh-event": Boolean(true), "monitoring-supported": Boolean(true), "state": String("valid"), "type": String("Temperature")}), Dictionary({"device-properties": Dictionary({"device-class": String("other"), "refresh-timeout": Integer(30)})})])})

Sorry, I don't currently have a laptop with NetBSD i.e. no battery to test with.

Cargo dependencies:
[dependencies]
nix = "*"
plist = "*"

I put the code here for easier testing.
Pull requests welcome!

I've extended the example code to include basic parsing of the structure and adding some example outputs, including one from a laptop with a battery. I think this now contains enough information that you should be able to extract the information you need rather easily. Can you take over from here?

A lot of project are still depending on this library instead of using Starship's fork.
I just made the full port for NetBSD.
https://github.com/starship/rust-battery/pull/69
Are you (and especially the maintainer) interested by a backport ?
I don't think it should be too hard.
The interest might be about the ffi that the Starship's fork removed.


Cannot implement it into project

I'm a beginner to swift 3 and cocoapods and i want to implement this https://github.com/luowenxing/MTImagePicker image picker for my project.
I've installed the pod into my project and am using the .xcworkspace file.
However, I'm facing problems as i've errors like:
No such module 'MTImagePicker'
Use of unresolved identifier 'MTImagePickerController'
Use of undeclared type 'MTImagePickerAssetsModel'
I've moved the MTImagePicker folder into my project as instructed.
But I'm still having this problem.
Any help is much appreciated.
Thank you so much!

The project is written in swift2.3 Syntax and has not been updated for swift3.0.
I will update for swift3.0 soon.

Update for swift3.0


edit Key Expression

@krishahn - can you resolve the conflicts? I think they are as a result from another PR.

closing due to merge conflict


support multiple headers

this will close the issue I opened #24

Codecov Report

Merging #25 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #25   +/-   ##
=======================================
  Coverage   77.59%   77.59%
=======================================
  Files           7        7
  Lines         281      281
=======================================
  Hits          218      218
  Misses         45       45
  Partials       18       18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d51dac...895f7e1. Read the comment docs.

@brenol Thanks for your contribution! This is a feature I wanted but hadn't gotten around to.
I could confirm to specify as many as needed by repeating the flag.
❯ go run main.go -H "foo:bar" -H "Bar:Foo" -H "foo:bar2" --debug http://host.xz
"header" http.Header{
  "foo": []string{
    "bar",
    "bar2",
  },
  "Bar": []string{
    "Foo",
  },
}


Backup all folders

The only part of this software that I don't like is that I have have to mark each folder that I want backed up.   If I create a new folder I'll have to go in and mark it for backup.   I want it to backup ALL the folders, even when I add new ones.
Even better would be an option for all folders with an exception list.

I'd welcome a PR for this :-)
I suppose a "select all" option in the config menu would work.


cleanups and DRYing up the reconciler unit tests

Adds a series of cleanups to the pkg/runtime/reconciler_test.go unit test to reduce
the amount of repetitive setup code we had in there.
Also removes the unused AWSResourceDescriptor.UpdateCRStatus interface method.
By submitting this pull request, I confirm that my contribution is made under the terms
of the Apache 2.0 license.

@vijtrip2 @RedbackThomson @A-Hilaly note that each commit in this PR is self-contained and separately revertable/cherry-pickable, so feel free to merge instead of squash-merge.


Switching mod to next device doesn't work

When I try to clear an mod+2 and pair a new device the device goes back mod+1 device though it shows blue color

It will show as connected to any paired device but will only output to the selected device

The problem is select mod +2 but it sends output to mod +1 while showing
the color of mod + 2
On Wed, Jul 5, 2023 at 1:24 PM ReFil @.***> wrote:

It will show as connected to any paired device but will only output to the
selected device
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1622445418,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHOXUAWVWA7DT6WCVGTXOXERNANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

--
Regards,
Venkata Sreekanth Bhagavatula

Does this happen with all the Bluetooth profiles? Have you tried another?

Yes, tried with Mod +3, same issue.
Regards,
Venkata Sreekanth Bhagavatula
On Thu, Jul 6, 2023 at 3:17 AM ReFil @.***> wrote:

Does this happen with all the Bluetooth profiles? Have you tried another?
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1623197263,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHJNLCJSUMMAN3MV2CTXOZX75ANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Is this an issue with a new keyboard, or had it been working fine until a firmware update?

The keyboard came with an older firmware, I upgraded it to version 2 with a
slight modification in keymap for mac. Otherwise nothing else changed. I
updated the firmware a couple of months ago.
Regards,
Venkata Sreekanth Bhagavatula
On Thu, Jul 6, 2023 at 11:11 AM Kinesis Keyboards @.***>
wrote:

Is this an issue with a new keyboard, or had it been working fine until a
firmware update?
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1623943648,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHM7EM4FXG6ZTVMMYODXO3PTZANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Understood. Did switching Profiles work for some period of time that after keymap update a couple of months ago?

I don't recall, but at some point I did switch between computers but I am
not sure if it was before or after the keymap/firmware update. I am in need
to switch between computers and devices now though.
Regards,
Venkata Sreekanth Bhagavatula
On Thu, Jul 6, 2023 at 11:20 AM Kinesis Keyboards @.***>
wrote:

Understood. Did switching Profiles work for some period of time that after
keymap update a couple of months ago?
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1623960474,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHOQEQF4KIEV7UBYUB3XO3QUFANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Just wondering if perhaps the BT Select commands may have been scrambled. If they haven't. It's probably best to perform a Settings Reset and install factory firmware to see if its a hardware defect.

For more personalized support, and if necessary a Return Merchandise Authorization, please submit a trouble ticket here:
https://kinesis-ergo.com/support/contact-a-technician/

Do you want me to install version 2 or version 3? when you say factory
reset, does it mean no keymap?
Regards,
Venkata Sreekanth Bhagavatula
On Thu, Jul 6, 2023 at 11:53 AM Kinesis Keyboards @.***>
wrote:

Just wondering if perhaps the BT Select commands may have been scrambled.
If they haven't. It's probably best to perform a Settings Reset and install
factory firmware to see if its a hardware defect.
[image: image]
https://user-images.githubusercontent.com/37431414/251518148-ec2329b4-c291-4415-a4e1-d6aca862c1f7.png
For more personalized support, and if necessary a Return Merchandise
Authorization, please submit a trouble ticket here:
https://kinesis-ergo.com/support/contact-a-technician/
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1624001481,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHOLPDNFZEIUN6SKGDLXO3UO5ANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Either version 2.0 or 3.0 should be sufficient to test for this issue. Yes, factory version means the default layout so you would lose your customizations. But it should be a simple matter to test and rebuild the f/w if necessary.

I will try it after work and let you know.
Regards,
Venkata Sreekanth Bhagavatula
On Thu, Jul 6, 2023 at 11:56 AM Kinesis Keyboards @.***>
wrote:

Either version 2.0 or 3.0 should be sufficient to test for this issue.
Yes, factory version means the default layout so you would lose your
customizations. But it should be a simple matter to test and rebuild the
f/w if necessary.
—
Reply to this email directly, view it on GitHub
https://github.com/KinesisCorporation/Adv360-Pro-ZMK/issues/179#issuecomment-1624005808,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2TSHK2CXUVIPXS2GIK4QDXO3U4XANCNFSM6AAAAAAZ7MLEE4
.
You are receiving this because you authored the thread.Message ID:
@.***>

Hi, following up on this issue, was the problem solved with the factory firmware?

Just following up on this, have you been able to solve this issue?

Hi, just following up one last time, if i dont hear back in 48h i'll close out this issue as solved


The parameter soapClientClassName

The parameter "soapClientClassName" with the value of the option "DEFAULT_SOAP_CLIENT_CLASS"
/**
 * @see SoapClient::__doRequest()
 */
class MySoapClient extends \SoapClient
{
    /**
     * @see SoapClient::__doRequest()
     */
    public function __doRequest($request, $location, $action, $version, $one_way = 0)
    {
        // Overwrite method
        return parent::__doRequest($request, $location, $action, $version, $one_way);
    }
}

/**
 * Minimal options
 */
$options = [
     WsdlToPhp\PackageBase\AbstractSoapClientBase::WSDL_URL => 'WSDL'
     WsdlToPhp\PackageBase\AbstractSoapClientBase::WSDL_CACHE_WSDL => true,
     WsdlToPhp\PackageBase\AbstractSoapClientBase::WSDL_SOAP_VERSION => SOAP_1_2,
     WsdlToPhp\PackageBase\AbstractSoapClientBase::WSDL_CLASSMAP => ClassMap::get(),
     WsdlToPhp\PackageBase\AbstractSoapClientBase::DEFAULT_SOAP_CLIENT_CLASS => MySoapClient::class, // USE SOAP CLASS
];

/**
 * Samples for Get ServiceType
 */
 $ws = new Get($options);

Hi,
What are you solving by this PR?
I'm not certain to understand or there is another work around that is already in place that would not required such code modification.
Let me know.

Hola,
¿Qué estás resolviendo con este PR?
No estoy seguro de entenderlo o hay otra solución que ya existe y que no requeriría dicha modificación de código.
Hágamelo saber.

Solo queria

Hi,
What are you solving by this PR?
I'm not certain to understand or there is another work around that is already in place that would not required such code modification.
Let me know.

I just wanted to use a modified clear of the SoapClient, I tried using the DEFAULT_SOAP_CLIENT_CLASS option but I see that this is never passed to the "soapClientClassName" method, if you can tell me what the other option would be, thanks

Hi,
What are you solving by this PR?
I'm not certain to understand or there is another work around that is already in place that would not required such code modification.
Let me know.

I just wanted to use a modified clear of the SoapClient, I tried using the DEFAULT_SOAP_CLIENT_CLASS option but I see that this is never passed to the "getSoapClientClassName" method, if you can tell me what the other option would be, thanks

Use the "setSoapClient" method as follows:
$ws = new Get($options);
$ws->setSoapClient(new MySoapClient('WSDL', [
    'trace' => true,
    'features' => 5,
    'cache_wsdl' => true,
    'exceptions' => true,
    'soap_version' => SOAP_1_2,
    'classmap' => ClassMap::get(),
]));

Here you're redefining the SoapClient instance whereas it has been created within the first line.
The PackageBase project is mainly intended to be used by the generated classes from the PackageGenerator project. Using the SoapClient option, you can then override properly the default SoapClient class in order to keep the native behaviour.

Here you're redefining the SoapClient instance whereas it has been created within the first line.
The PackageBase project is mainly intended to be used by the generated classes from the PackageGenerator project. Using the SoapClient option, you can then override properly the default SoapClient class in order to keep the native behaviour.

Thank you!


Implement number of records

The number of records comes from the main /antibodies api

Initialize with value coming from https://www.areg.dev.metacell.us/docs#/default/get_datainfo_api_datainfo_get


Implement initial KEL validator

validator was already implemented with the obi resolution. KEL go through the CESR parser that validates consistency. We check if the parser returns successfully.


Use HTTPS for s3 assets

Why?

Paperclip was returning http asset URLs, but production site forces HTTPS

What Changed?

Change paperclip to return https URLs.

Doing a staging deploy for this one right now.

CONFIRMED: This is working on staging. Just checked the URL of the tomato guide images.

Coverage remained the same when pulling ad63f54932f4b61d738ac961a4c89b81021af20b on RickCarlino:master into f130889b0bc49f2abd88ffc17810d82e84e51d19 on openfarmcc:master.

Thanks!


schematics, how it works?

I don't understand how to make it work, and there is no video on youtube, could you explain it to me?

Thanks


Run npm install to sync lock file with latest

What's this PR do?
This is the output of running npm install on master
Why are we doing this? How does it help us?
The hope is that every time you need to run npm install, you're not committing an unintentional change.
How should this be manually tested?
On master:
run make to get into the app and then npm install. git status should show a change in package.lock.json. Undo those changes to clean your staged commits.
In this branch:
Run npm install again. Confirm that package.lock.json remains unchanged.
How should this change be communicated to end users?
N/A
Are there any smells or added technical debt to note?
This may introduce a security warning about acorn in the GitHub interface. If that's the case we'll push up another PR with a manual npm audit fix and deal with that in each subsequent PR.
What are the relevant tickets?
N/A - Just trying to prevent weird future commits
Have you done the following, if applicable:
(optional: add explanation between parentheses)

[ ] Added automated tests? ( )
[ ] Tested manually on mobile? ( )
[ ] Checked BrowserStack? ( )
[ ] Checked for performance implications? ( )
[ ] Checked accessibility? ( )
[x] Checked for security implications? ( )
[ ] Updated the documentation/wiki? ( )

TODOs / next steps:

[ ] Determine if this creates a new security warning and run npm audit fix if so

:shipit:

@AndrewGibson27, I've been trying to verify that donations still submit properly on this. I'm getting successful submit, but it never seems to hit the test Slack channel. Should I be worried about that? I'm almost sure it's some env var I'm missing - looks like September was the last time that ever worked for me. I have ENABLE_SLACK, SLACK_API_KEY, and SLACK_CHANNEL all filled out so I'm not sure what's missing there.

Adding some Slack context to my previous comment. We determined this to be a bug and we'll consult @x110dc on what could be happening there.


Single-click Filemanager trigger and open 2 folders consecutively

Single clicking for the filemanager tree is currently broken, tested with Chrome and Firefox, caches cleaned, no plugins. It only responds to double clicking whatever option is selected.
Second issue is a long standing one. If you try to expand 2 folders consecutively, their contents overlap and show all together as they were inside the last clicked folder while the first clicked folder remains with the progress spinner.
But Codiad is known to have some quirks on specific server configurations so I might be the oddball here.

I have tried both on a fresh installation (apache 2.4 + 5.6.x) on a winnt based system with latest chrome x64 without any issue. could you check chrome's console log if there is any kind of error displayed?

Nothing in the Console and no open events in the Network tab. Chrome 40.0.2214.115 m, not sure what Apache but suspect 2.4 and PHP 5.5.20. Tried a fresh install, both abspath and relative project. Can't even single-click to switch projects. Nevermind it too much, Daeks, I'll try it again in a couple of versions. I keep coming back to it =)

Please reopen if required

regarding first issue ref. to https://github.com/Codiad/Codiad/commit/c04414a9dcb631119a28db416ed5d7f59bc8c09e

Confirmed to work :+1:
Thanks Daeks.