pcdshub / pytmc

Generate EPICS IOCs and records from TwinCAT projects - along with many TwinCAT project tools
https://pcdshub.github.io/pytmc/
Other
10 stars 11 forks source link

FIX: default io type should be io, as per the docs #103

Closed ZLLentz closed 5 years ago

ZLLentz commented 5 years ago

https://slaclab.github.io/pytmc/pragma_usage.html#io

teddyrendahl commented 5 years ago

Power cycling to get changes from #105

codecov-io commented 5 years ago

Codecov Report

Merging #103 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #103   +/-   ##
======================================
  Coverage    76.5%   76.5%           
======================================
  Files          14      14           
  Lines        1524    1524           
======================================
  Hits         1166    1166           
  Misses        358     358
Impacted Files Coverage Δ
pytmc/record.py 94.15% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b180596...86017bb. Read the comment docs.

klauer commented 5 years ago

... on second thought, let's discuss this...

ZLLentz commented 5 years ago

The alternate PR would be one that changes the docs appropriately.

teddyrendahl commented 5 years ago

I'd vote for having the default not be "writable". I think the consequences of accidentally exposing an internal field is greater than not having a field be writable.

That being said I'm not completely against @ZLLentz suggestion of removing the ambiguity and raising an error (or at least a warning if no io is specified)

ZLLentz commented 5 years ago

I think this is an important enough choice that I'd want it to have no default. If there was a default, I don't really care which we choose, I just want the docs and the code to be consistent. I wrote the ppm lib stuff assuming the io default, got burned, and I only want to rewrite it once.

klauer commented 5 years ago

@n-wbrown interesting - a prototyping mode could be useful. Thoughts @ZLLentz @teddyrendahl?

ZLLentz commented 5 years ago

I don't think adding an io line significantly slows down your prototyping speed. I wouldn't oppose these sorts of modes but I don't expect to get much use out of them personally.

n-wbrown commented 5 years ago

Merging to bring code behavior and docs in line with one another. We can certainly revisit and change this at a later date if anyone develops strong feelings for the topic.