Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate suggestions from #2578 #2579

Merged
merged 5 commits into from
Aug 10, 2020

Conversation

otronarp
Copy link
Member

Clarify that behavior is unspecified for:

  • Curves where x or y does not designate a variable
  • Unknown units or incompatible with the plotted variable

Clarify that behavior is unspecified for:
 - Curves where x or y does not designate a variable
 - Unknown units or incompatible with the plotted variable
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

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

Requesting changes, as I am afraid that an overuse of unspecified behavior in the specification will result in the development of non-portable figures. (A quick search for unspecified behavior in the specification revealed only one instance, being how a tool should deal with the entire Documentation annotation, and I don't think that is a good example of what we should be doing here.)

Note that what is much more common is to say that something is an error, implicitly leaving it up to the tools to decide how to deal with that error.

chapters/annotations.tex Outdated Show resolved Hide resolved
chapters/annotations.tex Outdated Show resolved Hide resolved
@HansOlsson HansOlsson added the MCP0033 Predefined plot issues and PRs (MCP-0033) label Jun 16, 2020
chapters/annotations.tex Outdated Show resolved Hide resolved
chapters/annotations.tex Outdated Show resolved Hide resolved
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

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

Approving, assuming there will be a separate PR for how to give meaning to an unspecified Axis.unit.

axis tick marks, so the axis \lstinline!label! shall not contain the unit.

If a tool does not recognize the \lstinline!unit!, it is recommended to issue a
warning and treat the \lstinline!unit! as if it was not specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I realize that this isn't really an option as long as the unit is mandatory. However, I believe that having a mandatory Axis.unit, which in turn causes Plot.x and Plot.y to be mandatory, is a problem that needs to be resolved separately, and that we open up a separate PR for how this should be done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That separate PR is now opened as #2632.

@henrikt-ma henrikt-ma merged commit 4370791 into modelica:MCP/0033 Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MCP0033 Predefined plot issues and PRs (MCP-0033)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants