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

CV - Merge Data with Images #157

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Rwinstonbnc
Copy link

Please include answers to these questions as part of your pull request

In the GitHub webUI, use the Write tab to modify the Markdown text that is part of the pull request. For each question simply place an X inside the square brackets, [X], that represents your answer. Make sure there are no blanks inside the brackets, otherwise MarkDown doesn't render properly. Using the Preview tab while editing this form, you can see the formatted/rendered version of the message.

  • Q1: Confirm that you have the right to submit the code that is being contributed. Please consider the origin of your code and confirm you have the appropriate rights to make the submission subject to the Apache 2.0 license that applies to everything in this repository of custom steps. If so, follow the instructions for the Contributor Agreement (which is based on the industry-standard Developer Certificate of Origin (DCO)).
    • Yes, I have the right to submit the contributed code on behalf of myself, my company, or any other owner of the code. I have also attached my signed copy of the DCO to this message.
    • No
  • Q2: Confirm that your contribution does not include any personally identifiable information (PII), for example, in any examples used in your README file.
    • My contribution does NOT include PII data
    • My contribution includes PII data
  • Q3: Confirm your contribution does not include any encryption or other export-controlled technology.
    • My contribution does NOT contain encryption or other export-controlled technology
    • My contribution includes encryption or other export-controlled technology
      ContributorAgreement.txt

@snlwih snlwih added the new custom step This pull request represents a new custom step label Jul 22, 2024
@snlwih snlwih changed the title Cv merge data with images CV - Merge Data with Images Jul 22, 2024
@snlwih
Copy link
Collaborator

snlwih commented Jul 24, 2024

Hi Robert, and the story continues with another Computer Vision related custom step 😄

As always here is my feedback:

  • Screenshots in readme missing 1-pixel black border
  • Labels for certain UI controls missing colon at end of the label
  • Codegen that is part of the SAS Studio Custom Step framework provides dedicated macro variables for libref and table (and more). So no need to extract that info the name of an inputtable or outputtable control, simply use inputtable_lib, ... When you look at the generated code when you use your custom step and select "Unfold All Regions" in the generated code, you can see what is macro variables are generated for each of the controls on your UI (or read the SAS Studio documentation ;-))
  • You should not assume that for a libref that points to a caslib the name of libref and caslib are the same. I've provided code snippets that explain how to retrieve that info from the dictionary tables in comments for your previous contributions.
  • Use consistent coding for writing errors to the SAS log and stopping further execution. Sometimes you write the log messages using %put, followed by a data null step with just an abort statement. Other times ou use put statements to write the log messages inside the data null step.
  • Inconsistent idents, but this time only 2 lines so quick and easy to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new custom step This pull request represents a new custom step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants