idaes-pse icon indicating copy to clipboard operation
idaes-pse copied to clipboard

Get rid of default in documentation

Open dallan-keylogic opened this issue 2 years ago • 1 comments

Fixes

Remove references to the default dictionary in process block documentation. I'm not sure the best way to convey to the reader that they're meant to input config entries like normal function args, but this is what it looks like now:

image

Fixes #972 .

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

dallan-keylogic avatar Sep 23 '22 20:09 dallan-keylogic

Codecov Report

Base: 70.16% // Head: 70.17% // Increases project coverage by +0.00% :tada:

Coverage data is based on head (2bb813c) compared to base (3450781). Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #973   +/-   ##
=======================================
  Coverage   70.16%   70.17%           
=======================================
  Files         399      399           
  Lines       64993    64993           
  Branches    12027    12027           
=======================================
+ Hits        45600    45606    +6     
+ Misses      17057    17056    -1     
+ Partials     2336     2331    -5     
Impacted Files Coverage Δ
idaes/core/base/process_block.py 100.00% <ø> (ø)
idaes/ver.py 61.53% <0.00%> (-4.62%) :arrow_down:
...daes/apps/roundingRegression/RoundingRegression.py 84.69% <0.00%> (+1.02%) :arrow_up:
idaes/core/dmf/util.py 55.55% <0.00%> (+1.19%) :arrow_up:
idaes/core/dmf/codesearch.py 100.00% <0.00%> (+1.44%) :arrow_up:
idaes/conftest.py 82.92% <0.00%> (+4.87%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Sep 23 '22 21:09 codecov[bot]

@dallan-keylogic What is the status on this PR? We probably want to get this into the November release so that our documentation is up-to-date for the transition.

andrewlee94 avatar Nov 14 '22 14:11 andrewlee94

@andrewlee94 I can take a look at it today. I still need to figure out what sort of substitution is going on when documents are built (probably via scripts @eslickj wrote), and then what is the best way to convey to the user that they should input config arguments like normal function keyword arguments.

dallan-keylogic avatar Nov 14 '22 14:11 dallan-keylogic

What happens if you make _config_block_keys_docstring = "{}"? Seems like that would remove the "Config args" heading. Then you may need to adjust the indentation of the autogenerated config block docks. I'm not totally sure but I think that should make them look like the rest of the args.

eslickj avatar Nov 15 '22 16:11 eslickj

I guess without generating the documentation, you should be able to print the __doc__ string for an IDAES model class and make adjustments. I can probably make it look right, so let me know if you have any trouble and I'll mess with it.

eslickj avatar Nov 16 '22 14:11 eslickj

@dallan-keylogic @eslickj What is the status of the PR? This is one that would be good to get into the release (if we haven't already missed the bus).

andrewlee94 avatar Nov 29 '22 17:11 andrewlee94

@andrewlee94 , I'm taking another look at this one today. Managed to smooth out the config arguments, but still need to change formatting from headings to bullet points:

image

Looking at this documentation again, I don't think we ever want the user to use any of the first three arguments. Below the config arguments, we have another couple of arguments where we map config dictionaries to indices when creating an indexed unit.

image

Obviously the "default" reference needs to go, but in order for these arguments to make sense we need to maintain some sort of distinction between config arguments and non-config keyword arguments.

dallan-keylogic avatar Nov 29 '22 18:11 dallan-keylogic

@dallan-keylogic Whilst we do want to get the formatting sorted out, for this release I think we should focus on making sure we get rid of the references to default so that out docs are correct (if poorly formatted).

andrewlee94 avatar Nov 29 '22 18:11 andrewlee94

I reverted us back to having Config be a special header for now, but got rid of the additional "default" hiding in the arguments below the config section.

image

dallan-keylogic avatar Nov 29 '22 18:11 dallan-keylogic