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

[DYN-7923] CurveMapper node - for testing behavior #15776

Open
wants to merge 55 commits into
base: master
Choose a base branch
from

Conversation

ivaylo-matov
Copy link
Contributor

@ivaylo-matov ivaylo-matov commented Jan 22, 2025

Purpose

Draft PR for @achintyabhat to test behavior.

🚧 Note: This is not intended for merging as the code is still in a rough state. Once the expected behavior is confirmed, I will create a clean branch and move the model and control outside of charts.

@QilongTang, I hope this works for now. If not, let me know, and I can create a new branch right away.

PS: One issue I am aware of and will fix next is that the node executes the default values on the first manual graph run, even when the inPorts are connected.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Draft PR for @achintyabhat to test behavior.

🚧 Note: This is not intended for merging as the code is still in a rough state. Once the expected behavior is confirmed, I will create a clean branch and move the model and control outside of charts.

@QilongTang, I hope this works for now. If not, let me know, and I can create a new branch right away.

PS: One issue I am aware of and will fix next is that the node executes the default values on the first manual graph run, even when the inPorts are connected.

Reviewers

@QilongTang

FYIs

@achintyabhat
@dnenov

ivaylo-matov and others added 30 commits December 9, 2024 19:05
The node shows in Dynamo with inPorts and outPorts rendered
coordinate system, imput values and resizing works
Comments from catchup implemented
- curve mapper node now shows up in the library
- icons working!
…om/ivaylo-matov/Dynamo into DYN-7923-Curve-Mapper-node"

This reverts commit 82a7a9c5af25f610a7e335e967d63aeb2cd46bcc, reversing
changes made to 2bce244.
need to fix fonts
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7923

@QilongTang QilongTang added DNM Do not merge. For PRs. WIP labels Jan 23, 2025
private Point point;
private bool isEnabled = true;

public bool IsOrthogonal { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing comments for some public properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, I'll go over all and add the comments. Need to register in the API as well. Likely that I will rewrite all in clean branch.

@@ -45,7 +45,7 @@
// to distinguish one build from another. AssemblyFileVersion is specified
// in AssemblyVersionInfo.cs so that it can be easily incremented by the
// automated build process.
[assembly: AssemblyVersion("3.5.0.6885")]
[assembly: AssemblyVersion("3.5.0.7603")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not commit this file.

Background="Transparent"
WindowStyle="None">
<dww:ModelessChildWindow
x:Class="PythonNodeModelsWpf.ScriptEditorWindow"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have changes to python script editor here? Looks like just alignment and white space changes. Are you using a different VS setting?

@@ -24,6 +24,8 @@
<None Remove="Resources\alert.png" />
<None Remove="Resources\convert.png" />
<None Remove="Resources\convert_hover.png" />
<None Remove="Resources\lock_dafault_16px.png" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Confused as to why these resources are added to this project.

return (GraphTypes)parameter;
}
}
public class EnumBindingSourceExtension : MarkupExtension
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing comments for public classes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM Do not merge. For PRs. WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants