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

Updated docstrings in pymc.model.core.Model #7118

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

apalermo01
Copy link
Contributor

@apalermo01 apalermo01 commented Jan 27, 2024

Description

First time contributor here (both to pymc and open source) - updated docstrings in pymc.model.core.Model to follow numpydoc.

There are some details that I need to call out. Some of these are notes that the instructions asked to raise (1-3), some are questions I have since I'm still unfamiliar with the codebase (4-9):

  1. Should rv_var in these function be a TensorVariable or tensor_like?

    • make_obs_var
    • create_value_var
    • register_rv
  2. I found ndarray in the input arguments for these functions:

    • make_obs_var
    • compile_fn (in return type)
    • update_start_vals
    • initial_point
    • set_data
    • replace_rvs_by_value (in return type)
  3. The value_var parameter is marked as a Variable - can I get a confirmation that this should not be RandomVariable?

  4. There weren't any type hints in these functions. I think I can infer some of these, but I think changes like that would be out of scope for this PR.

    • __init__
    • logp_dlogp_function
    • shape_from_dims
    • set_initval
    • register_rv
    • make_obs_var (dims only)
    • add_named_variable (just var - should this be Variable?)
    • profile
    • point_logps
  5. There originally weren't any parameters documented or type hints for these functions.

    • shape_from_dims
    • set_initval
  6. These functions didn't have a parameters section.

    • create_value_var
    • add_named_variable
    • update_start_vals
  7. In register_rv - what should the datatypes be for transform and initval? Additionally, I'm not clear on what UNSET is doing.

  8. In the add_coords function - should Model.add_coord be single or double backticks?

  9. In compile_fn, what's the datatype for mode? I'm assuming its a string. Digging into pytensor it looks like the options for mode are "FAST_COMPILE", "FAST_RUN", "DebugMode", "NanGuardMode", and "DEBUG_MODE".

Related Issue

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

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

Copy link

welcome bot commented Jan 27, 2024

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.

@twiecki
Copy link
Member

twiecki commented Jan 27, 2024

Great first contribution @apalermo01 - thanks!

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2da4050) 92.21% compared to head (1f8656b) 88.61%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7118      +/-   ##
==========================================
- Coverage   92.21%   88.61%   -3.61%     
==========================================
  Files         101      101              
  Lines       16901    16901              
==========================================
- Hits        15586    14977     -609     
- Misses       1315     1924     +609     
Files Coverage Δ
pymc/model/core.py 91.95% <ø> (-0.26%) ⬇️

... and 8 files with indirect coverage changes

@ricardoV94 ricardoV94 merged commit d95acc0 into pymc-devs:main Mar 4, 2024
23 of 24 checks passed
Copy link

welcome bot commented Mar 4, 2024

Congratulations Banner]
Congrats on merging your first pull request! 🎉 We here at PyMC are proud of you! 💖 Thank you so much for your contribution 🎁

@ricardoV94 ricardoV94 added the docs label Mar 4, 2024
@ricardoV94
Copy link
Member

@apalermo01 sorry for the delay. I am merging this because the current changes are already useful. Let me know if you are still interested in tackling the other issues you mentioned and I will try and respond to them.

Thanks!

mkusnetsov pushed a commit to mkusnetsov/pymc that referenced this pull request Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants