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

add automated container build to provide the library #1

Closed
vsoch opened this issue Aug 17, 2018 · 20 comments
Closed

add automated container build to provide the library #1

vsoch opened this issue Aug 17, 2018 · 20 comments

Comments

@vsoch
Copy link
Member

vsoch commented Aug 17, 2018

@earcanal you can use the configuration at expfactory-experiments/state-mindfulness-survey#3 to get this setup. I can take care of connecting to CircleCI - could you please look at the error in the link about root install and help debug what we should try? What I'd like to do is:

  1. Get that build setup here to build (and deploy) the expfactoryR container
  2. Then use expfactoryR in that linked PR so we don't have to build it
@vsoch
Copy link
Member Author

vsoch commented Aug 17, 2018

okay I'm doing some testing in the survey repo to see if I can install expfactoryR THEN the functions in the experiment, something like:

            echo "Installing package efsmsr"
            R -e "devtools::install_github('expfactory/expfactoryR')"
            #R -e "devtools::install_local('efsmsr')"
            cd /tmp/src/efsmsr
            R -e 'devtools::check()'
            R -e 'devtools::test()'

I'm sure it will fail at least 43 times before it gets working, I'm ok with this :)

@vsoch
Copy link
Member Author

vsoch commented Aug 17, 2018

And what does "efsmsr" mean?

@vsoch
Copy link
Member Author

vsoch commented Aug 17, 2018

yep that worked! Wicked cool :) I think we should add the same install routine here, to test the expfactoryR package itself. Did you write tests for it?

@earcanal
Copy link
Collaborator

Yes, there are tests for expfactoryr.

The namespace constraints in R are quite restrictive, so I've gone for the general pattern ef for experiment factory, followed by some sort of abbreviation of the experiment/survey, followed by r. Hence efsmsr.

@vsoch
Copy link
Member Author

vsoch commented Aug 22, 2018

why not be more explicit and go for the whole string at least, e.g., expfactory-stroop and have it be an r package? Why does it have to be short, and why does it have to end in "R." ?

@earcanal
Copy link
Collaborator

earcanal commented Aug 22, 2018

  • ef was to save typing, but yes expfactory might be clearer
  • "the name can only consist of letters, numbers and periods" i.e. no dashes

Given the above, if we break the allowed but discouraged . ("I recommend against using periods in package names because it has confusing connotations ") we could have

  • expfactory.stroop
  • expfactory.breath.counting (a bit clunky for multi-word tasks)

@earcanal
Copy link
Collaborator

The more I read the rulez, the more they seem to have been designed to obfuscate the relationship between a package name and what it does! My love-hate relationship with R continues to unfold... :)

@vsoch
Copy link
Member Author

vsoch commented Aug 22, 2018

I am ok with mostly anything, but I think the package should have some logical underpinning. So "efsmsr" is not great, but any of the other ones above would be ok.

@earcanal
Copy link
Collaborator

Authors seem to ignore the no CamelCase advice, so maybe expfactoryCamelCaseTaskName, which would give

  • expfactory (core library)
  • expfactoryStroop
  • expfactoryBreathCounting
  • expfactoryStateMindfulnessScale
  • expfactoryFiveFacetMindfulnessQuestionnaire
  • expfactoryAttentionNetworkTest
  • ...

@vsoch
Copy link
Member Author

vsoch commented Aug 22, 2018

What's wrong with the dot one, and then we would have the expfactory namespace for all experiments?

@earcanal
Copy link
Collaborator

From packages names I see on CRAN, nobody seems to break the "no dots" rule, but many break the "no caps" rule. Maybe I should ask about relative merits on Stack Overflow.

@vsoch
Copy link
Member Author

vsoch commented Aug 22, 2018

In Python a "dot" would come down to a different library (meaning if the library is called "expfactory" and you do from expfactory.utils import that would come down to a top level folder called expfactory with a module (file) called utils.py in the folder. I think what we would want to do is have it so that if a user installs an expfactory module, it's sort of a "submodule" for some parent namespace (expfactory). Like:

expfactory/
    stroop
    breatheCountingTask

etc.

@earcanal
Copy link
Collaborator

Yeah, that would be great ... if R had the concept of hierarchical namespaces which mapped nicely onto filesystems. AIUI it has neither. It just dumps all packages into (global or local) lib dirs. I imagine this is the origin of the "no dots" rule, i.e. don't make namespaces look hierarchical when/because they're not. Breaking the CamelCase rule makes sense to me so that package names are easier to read, at the expense of being harder to type.

@vsoch
Copy link
Member Author

vsoch commented Aug 22, 2018

Why not break the rule with dots (or some other allowable character?) It looks nicer, more pythonic :)

@earcanal
Copy link
Collaborator

I wonder whether the rules are then specifically to prevent it looking like something it isn't i.e. something moRonic. I'll take a reading from SO.

@earcanal
Copy link
Collaborator

I've asked this question on SO.

@earcanal
Copy link
Collaborator

earcanal commented Aug 24, 2018

Hmm. It got put on hold because it was deemed the answers would be opinion. I don't think I can re-word it to remove the hold.

There are groups of packages that use ., e.g. assertive.*. I've emailed one of the authors to get an opinion.

@richierocks
Copy link

The assertive naming scheme arose because it was originally a single package. It grew too big, so I split it into several smaller packages. Now assertive itself just imports and reexports all the functions from each of the assertive.* packages. The idea is that if you are making your own package, you want the smallest possible set of dependencies, you you can depend upon only the assertive.* functions that you use. By contrast, if you are working interactively, you don't want to have to think about which assertive.* package the function you want is contained in, so you can just write library(assertive) and get everything. I used the same tactic with the rebus package.

As a general principle, using lowercase names makes them easier to type. I'd favor something like

expfactory.base (core library)
expfactory.stroop
expfactory.breathcounting
expfactory.statemindfulnessscale
expfactory.fivefacetmindfulnessquestionnaire
expfactory.attentionnetworktest

@vsoch
Copy link
Member Author

vsoch commented Aug 24, 2018

I'm huge +1 to what @richierocks has to say! I like the examples provided (with . and no weird capitalization) and think we should do exactly that.

@earcanal
Copy link
Collaborator

OK. I'll initiate a mass renaming ASAP. :)

earcanal added a commit to earcanal/state-mindfulness-survey that referenced this issue Aug 24, 2018
earcanal added a commit to earcanal/breath-counting-task that referenced this issue Aug 24, 2018
earcanal added a commit to earcanal/attention-network-task that referenced this issue Aug 24, 2018
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

No branches or pull requests

3 participants