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

8461 non expressive variable name #8504

Merged
merged 9 commits into from
May 3, 2021

Conversation

joaoCeilandia
Copy link
Contributor

Proposed changes:
-fix #8461

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@sara-tagger
Copy link
Collaborator

Thanks for submitting a pull request 🚀 @ArjaanBuijk will take a look at it as soon as possible ✨

@@ -563,8 +563,8 @@ def __init__(
self._layer_norm = tf.keras.layers.LayerNormalization(epsilon=1e-6)

def _get_angles(self) -> np.ndarray:
i = np.arange(self.units)[np.newaxis, :]
return 1 / np.power(10000, (2 * (i // 2)) / np.float32(self.units))
array_2D = np.arange(self.units)[np.newaxis, :]
Copy link
Contributor

Choose a reason for hiding this comment

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

please use all lowercase: array_2d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

i = np.arange(self.units)[np.newaxis, :]
return 1 / np.power(10000, (2 * (i // 2)) / np.float32(self.units))
array_2D = np.arange(self.units)[np.newaxis, :]
return 1 / np.power(10000, (2 * (array_2D // 2)) / np.float32(self.units))
Copy link
Contributor

Choose a reason for hiding this comment

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

please use all lowercase: array_2d

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Contributor

@ArjaanBuijk ArjaanBuijk left a comment

Choose a reason for hiding this comment

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

To be pep8 compliant, please use all lowercase for the new variable name.

Copy link
Contributor

@ArjaanBuijk ArjaanBuijk left a comment

Choose a reason for hiding this comment

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

Thank you!

@ArjaanBuijk ArjaanBuijk added the area:rasa-oss/ml 👁 All issues related to machine learning label Apr 26, 2021
@ArjaanBuijk ArjaanBuijk added this to the 2.6 Rasa Open Source milestone Apr 26, 2021
@ArjaanBuijk
Copy link
Contributor

@joaoCeilandia ,
All is ready to be merged, but your branch is out-of-date with the base branch, so I cannot merge it.
Can you please update the branch in your fork, or make your branch writeable for me?

@joaoCeilandia
Copy link
Contributor Author

Has no conflicts with the base branch now. I don't know if i can make my branch writeable for you because it is from a class in my college. @ArjaanBuijk

@ArjaanBuijk ArjaanBuijk enabled auto-merge May 3, 2021 13:08
@ArjaanBuijk
Copy link
Contributor

@joaoCeilandia ,
Can you bring your branch up-to-date with the base branch again?

sorry it has to be done this way, but if you are not able to make your branch writeable for me, you will have to update it manually again.

Things are added quickly, and your PR is in the queue to be merged in, but it needs to be squeezed in there.

@ArjaanBuijk ArjaanBuijk merged commit a039962 into RasaHQ:main May 3, 2021
@ArjaanBuijk
Copy link
Contributor

@joaoCeilandia ,
thanks for the PR!

@ErickGiffoni ErickGiffoni deleted the 8461-non-expressive-variable-name branch August 5, 2021 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss/ml 👁 All issues related to machine learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non expressive variable name on transformer.py
3 participants