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

Rectify Return type hint #7013

Closed
wants to merge 1 commit into from

Conversation

vivek-anand-singh
Copy link
Contributor

@vivek-anand-singh vivek-anand-singh commented Nov 14, 2023

Fixes #6811

Rectify Return type hint for find_measurable_comparisons


📚 Documentation preview 📚: https://pymc--7013.org.readthedocs.build/en/7013/

Copy link

welcome bot commented Nov 14, 2023

Thank You Banner
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

Copy link
Member

@larryshamalama larryshamalama left a comment

Choose a reason for hiding this comment

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

Hi @vivek-anand-singh, thanks for your PR! Apologies for getting back to you late. Please don't hesitate to ping me (or any devs you want to review your PR) via the comment section next time :)

There are some other places where we would need to rectify type hints. Would you like to include those changes in this PR? I found the occurrences for your convenience

def find_measurable_specify_shapes(fgraph, node) -> Optional[List[MeasurableSpecifyShape]]:

def find_measurable_check_and_raise(fgraph, node) -> Optional[List[MeasurableCheckAndRaise]]:

def find_measurable_cumsums(fgraph, node) -> Optional[List[MeasurableCumsum]]:

def find_measurable_clips(fgraph: FunctionGraph, node: Node) -> Optional[List[MeasurableClip]]:

def find_measurable_roundings(fgraph: FunctionGraph, node: Node) -> Optional[List[MeasurableRound]]:

def find_measurable_bitwise(fgraph: FunctionGraph, node: Node) -> Optional[List[MeasurableBitwise]]:

pymc/pymc/logprob/tensor.py

Lines 200 to 202 in f8b142a

def find_measurable_stacks(
fgraph, node
) -> Optional[List[Union[MeasurableMakeVector, MeasurableJoin]]]:

When you would like for me to have a look again, you can request my revision around the top right of this PR

@vivek-anand-singh
Copy link
Contributor Author

pymc/pymc/logprob/tensor.py

Lines 200 to 202 in f8b142a

def find_measurable_stacks(
fgraph, node
) -> Optional[List[Union[MeasurableMakeVector, MeasurableJoin]]]:

Hi @larryshamalama Thanks for the feedback ! should we change both MeasurableMakeVectorand MeasurableJoin to TensorVariable?

@larryshamalama
Copy link
Member

Hi @vivek-anand-singh, so sorry for the delay, holidays, travels and studies came in the way of my OSS involvement... You can perhaps just do:

def find_measurable_stacks( 
     fgraph, node 
 ) -> Optional[List[TensorVariable]]: 

FYI: this issue is being worked on in #7093. Because time has passed (which is my fault), I will add both of you as co-authors to whichever PR gets merged.

@vivek-anand-singh
Copy link
Contributor Author

Thanks for the response, Actually my exams are going on so I will do this after 2 days

@larryshamalama
Copy link
Member

#7093 is pretty much completed, so we are probably going to merge that PR and close this one. Again, apologies for the mix of delays; in principle, we should prioritize preceding PRs but there was a miscommunication here. We can add you as a co-author when merging the other PR.

@ricardoV94
Copy link
Member

Added as a co-author in fb1c48f

Sorry for the trouble!

@ricardoV94 ricardoV94 closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rectify Return type hint
3 participants