search

bluesky / suitcase-core

data export facilities for NSLS-II
https://blueskyproject.io/suitcase
Other
2 stars 13 forks source link

DEV: use data type #23

Closed licode closed 8 years ago

licode commented 8 years ago

Correction based on https://github.com/NSLS-II/suitcase/pull/21

codecov-io commented 8 years ago

Current coverage is 96.38% (diff: 99.07%)

Merging #23 into master will increase coverage by 0.39%

@@             master        #23   diff @@
==========================================
  Files             4          5     +1   
  Lines           848        858    +10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            814        827    +13   
+ Misses           34         31     -3   
  Partials          0          0

Powered by Codecov. Last update 5daa5da...b0c0e45

licode commented 8 years ago

Change on mds is updated for suitcase. more comments?

danielballan commented 8 years ago

I see that you added a metadatastore argument to the insert functions. This is a good change. You made it optional with a default value of db from from databroker import db. Unfortunately, if there are no global configuration files, that import will fail. If we want these insert functions to work going forward, I think the only safe thing to do is make the metadatastore argument required with no default.

Also, as a minor point, the name "mdsc" comes from "metadatastore.commands" which is now gone. Might as well name your new parameter mds, the same as we have named it elsewhere.

licode commented 8 years ago

Thanks for the suggestions. Also doc needs to be improved.

licode commented 8 years ago

remove default on db.mds. done.

licode commented 8 years ago

I think this is ready to go.

licode commented 8 years ago

more comments on this? @danielballan

danielballan commented 8 years ago

Sorry for the delay, @licode. Fix the one syntax problem and then self-merge this at will.

licode commented 8 years ago

good catch. thanks!